On Wed, Apr 29, 2020 at 9:29 AM Joe Orton <[email protected]> wrote: > > In uldap_connection_unbind, apr_ldap_rebind_remove() is always passed > NULL since ldc->ldap is either NULL on entry or is set to NULL. This > looks safe, but seems like an expensive noop since > apr_ldap_rebind_remove() acquires a mutex and iterates a linked list > trying to find a rebind reference matching NULL (I assume always > failing). > > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?revision=1876599&view=markup#l222 > > Can this be removed or should it be moved up so it's not a noop? I've > not tried to reproduce problems in an LDAP config with referrals. > > (Credit to Abhishek and Angel who saw apr_ldap_rebind_remove() spinning > and debugged this) > > Index: modules/ldap/util_ldap.c > =================================================================== > --- modules/ldap/util_ldap.c (revision 1877157) > +++ modules/ldap/util_ldap.c (working copy) > @@ -225,6 +225,12 @@ > > if (ldc) { > if (ldc->ldap) { > + /* forget the rebind info for this conn */ > + if (ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) { > + apr_ldap_rebind_remove(ldc->ldap); > + apr_pool_clear(ldc->rebind_pool); > + } > + > if (ldc->r) { > ap_log_rerror(APLOG_MARK, APLOG_TRACE5, 0, ldc->r, "LDC %pp > unbind", ldc); > } > @@ -233,11 +239,6 @@ > } > ldc->bound = 0; > > - /* forget the rebind info for this conn */ > - if (ldc->ChaseReferrals == AP_LDAP_CHASEREFERRALS_ON) { > - apr_ldap_rebind_remove(ldc->ldap); > - apr_pool_clear(ldc->rebind_pool); > - } > }
+1 to moving up
