Re: [tpop3d-discuss] Memory leak?

[Chris Lightfoot]

On Thu, May 09, 2002 at 03:07:10PM -0700, Marc Lewis wrote:
> On Thu, May 09, 2002 at 09:14:03PM +0100, Chris Lightfoot wrote:
> > On Thu, May 09, 2002 at 12:08:41PM -0700, Marc Lewis wrote:
> > > On Thu, May 09, 2002 at 11:20:45AM +0100, Chris Lightfoot wrote:
> >     [...]
> > > > Cheers. I'll see if I can reproduce anything with my test
> > > > OpenLDAP installation.
> > > 
> > > I'm very interested to hear your results.  
> > 
> > OK, I've discovered a few SNAFUs; the following patch
> > ought to fix them:
> [snip]
> 
> Well, I applied the patch, started up my expect script, after 113
> successful authentications (no failures), the program died:
> 
> auth_ldap_new_user_pass: ldap_search_s: Can't contact LDAP server

I still don't understand this. The example code in the
openldap distribution doesn't seem to handle this
specially.

> tpop3d: getentry.c:46: ldap_next_entry: Assertion `entry != ((void *)0)'
> failed.quit: signal 6 post_fork = 0
> Aborted

Well, the patch was a bit bogus. Try the following instead
(it's against the original source code):

retrieving revision 1.5
diff -u -r1.5 auth_ldap.c
--- auth_ldap.c 2002/02/25 18:16:05     1.5
+++ auth_ldap.c 2002/05/09 22:12:35
@@ -341,11 +341,10 @@
                 group = xstrdup(*vals);
 
             ldap_value_free(vals);
+            ldap_memfree(attr);
         }
 
-        /* Do NOT need to free the ber, apparently-- ldap_next_attribute frees
-         * it when it returns NULL. */
-        if (attr) ldap_memfree(attr);
+        ber_free(ber, 0);
 
         /* Check that we've retrieved all the attributes we need. */
 #define GOT_ATTR(a)     if (ldapinfo.attr.##a && !a) { \
@@ -393,9 +392,11 @@
     }
 
 fail:
+    /* Ugly: force the LDAP library to free user_addr. */
+    if (user_attr) while (ldap_next_entry(ldapinfo.ldap, ldapres));
+    
     if (ldapres) ldap_msgfree(ldapres);
-/*    if (user_attr) ldap_msgfree(user_attr);*/
-    if (user_dn) free(user_dn);
+    if (user_dn) ldap_memfree(user_dn);
 
     xfree(filter);
 
@@ -405,7 +406,7 @@
 /* auth_ldap_close:
  * Close the ldap connection. */
 void auth_ldap_close() {
-  if (ldapinfo.ldap) ldap_unbind(ldapinfo.ldap);
+    if (ldapinfo.ldap) ldap_unbind(ldapinfo.ldap);
 }
 
 /* auth_ldap_postfork:



You could try the following patch, which will force the
authenticator to reconnect to the server on every
authentication -- this is not much tested but should work.
It will make things slower, but I'm not sure by how much.

diff -u -r1.6 auth_ldap.c
--- auth_ldap.c 2002/05/09 22:26:49     1.6
+++ auth_ldap.c 2002/05/09 22:34:44
@@ -221,8 +221,6 @@
 
     r = 1;
 
-    auth_ldap_connect();
-
 fail:
     return r;
 }
@@ -245,10 +243,20 @@
     char *user_dn = NULL;
     int nentries, ret, i;
 
+    /* Connect to the server. */
+    for (i = 0; i < 3; ++i)
+        if (auth_ldap_connect()) {
+            if ((ret = ldap_simple_bind_s(ldapinfo.ldap, ldapinfo.searchdn, 
ldapinfo.password)) != LDAP_SUCCESS) {
+                log_print(LOG_ERR, "auth_ldap_new_user_pass: 
ldap_simple_bind_s: %s", ldap_err2string(ret));
+                ldap_unbind(ldapinfo.ldap);  /* not much we can do if this 
fails.... */
+                ldapinfo.ldap = NULL;
+            } else break;
+        }
+    
     /* Give up if there is no connection available. */
     if (!ldapinfo.ldap) {
         log_print(LOG_ERR, _("auth_ldap_new_user_pass: no connection to LDAP 
server"));
-        return NULL;
+        goto fail;
     }
 
     /* Obtain search filter. */
@@ -258,22 +266,6 @@
     if (verbose)
         log_print(LOG_DEBUG, _("auth_ldap_new_user_pass: LDAP search filter: 
%s"), filter);
 
-    /* Try to bind to the LDAP server, reconnecting if it's gone away. */
-    for (i = 0; i < 3; ++i) {
-        if (ldapinfo.ldap && (ret = ldap_simple_bind_s(ldapinfo.ldap, 
ldapinfo.searchdn, ldapinfo.password)) != LDAP_SUCCESS) {
-            log_print(LOG_ERR, "auth_ldap_new_user_pass: ldap_simple_bind_s: 
%s", ldap_err2string(ret));
-            ldap_unbind(ldapinfo.ldap);  /* not much we can do if this 
fails.... */
-            ldapinfo.ldap = NULL;
-        } else break;
-        if (!ldapinfo.ldap)
-            auth_ldap_connect();
-    }
-
-    if (!ldapinfo.ldap) {
-        log_print(LOG_ERR, _("auth_ldap_new_user_pass: unable to connect to 
LDAP server"));
-        goto fail;
-    }
-
     /* Look for DN of user in the directory. */
     if ((ret = ldap_search_s(ldapinfo.ldap, ldapinfo.dn, LDAP_SCOPE_SUBTREE, 
filter, NULL, 0, &ldapres)) != LDAP_SUCCESS) {
         log_print(LOG_ERR, "auth_ldap_new_user_pass: ldap_search_s: %s", 
ldap_err2string(ret));
@@ -400,13 +392,18 @@
 
     xfree(filter);
 
+    auth_ldap_close();
+
     return a;
 }
 
 /* auth_ldap_close:
  * Close the ldap connection. */
 void auth_ldap_close() {
-    if (ldapinfo.ldap) ldap_unbind(ldapinfo.ldap);
+    if (ldapinfo.ldap) {
+        ldap_unbind(ldapinfo.ldap);
+        ldapinfo.ldap = NULL;
+    }
 }
 
 /* auth_ldap_postfork:

-- 
``What is a committee?  A group of the unwilling,
  picked from the unfit, to do the unnecessary.''  (Richard Harkness)