On 11/07/2007 03:43 PM, [EMAIL PROTECTED] wrote: > Author: covener > Date: Wed Nov 7 06:43:26 2007 > New Revision: 592764 > > URL: http://svn.apache.org/viewvc?rev=592764&view=rev > Log: > Stop registering a cleanup on each LDAP connection created, this cleanup was > never called because it's registered against pconf in the child. LDAP > connections are created in the child and not shared between children, so no > action should be required at child exit > > Additionally, clarify comments around uldap_connection_cleanup() > > > Modified: > httpd/httpd/trunk/include/util_ldap.h > httpd/httpd/trunk/modules/ldap/util_ldap.c > > Modified: httpd/httpd/trunk/include/util_ldap.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_ldap.h?rev=592764&r1=592763&r2=592764&view=diff > ============================================================================== > --- httpd/httpd/trunk/include/util_ldap.h (original) > +++ httpd/httpd/trunk/include/util_ldap.h Wed Nov 7 06:43:26 2007 > @@ -190,8 +190,7 @@ > * Cleanup a connection to an LDAP server > * @param ldc A structure containing the expanded details of the server > * that was connected. > - * @tip This function is registered with the pool cleanup to close down the > - * LDAP connections when the server is finished with them. > + * @tip The connection is unlocked and returned to the list of free > connections > * @fn apr_status_t util_ldap_connection_cleanup(util_ldap_connection_t *ldc) > */ > APR_DECLARE_OPTIONAL_FN(apr_status_t,uldap_connection_cleanup,(void *param)); > > Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=592764&r1=592763&r2=592764&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original) > +++ httpd/httpd/trunk/modules/ldap/util_ldap.c Wed Nov 7 06:43:26 2007 > @@ -171,9 +171,8 @@ > > > /* > - * Clean up an LDAP connection by unbinding and unlocking the connection. > - * This function is registered with the pool cleanup function - causing > - * the LDAP connections to be shut down cleanly on graceful restart. > + * Clean up an LDAP connection by unbinding and unlocking the connection, > + * causing it to be returned to the list of free connections > */ > static apr_status_t uldap_connection_cleanup(void *param) > { > @@ -562,11 +561,6 @@ > > /* save away a copy of the client cert list that is presently valid > */ > l->client_certs = apr_array_copy_hdr(l->pool, st->client_certs); > - > - /* add the cleanup to the pool */ > - apr_pool_cleanup_register(l->pool, l, > - uldap_connection_cleanup, > - apr_pool_cleanup_null);
Hm. Currently this works fine, but in r591488, you create a subpool of st->pool and save it in l->pool and I understood that it was your intention that this subpool will be cleared sometime in the future and at this point of time uldap_connection_cleanup seems to be important again. As I see no harm in registering the cleanup for this pool and as the person who introduces clearing this subpool will have forgotten about this cleanup I would think that reverting would be the best here. Regards Rüdiger
