[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: close(-1) in HEAD

I figured this was harmless. Another alternative is to leave the sockbuf as it was, and pass it into slap_sd_remove() to let it get freed there. I suppose that would be less messy.

Pierangelo Masarati wrote:
Recently, a number of close(-1) started occurring in HEAD while shutting down connections. I noticed them because of valgrind, and I tracked it to Howard's recent change to connection_destroy() where the the descriptor is invalidated and the sockbuf is then closed:

       ber_sockbuf_ctrl( sb, LBER_SB_OPT_GET_FD, &sd );

       ber_sockbuf_ctrl( sb, LBER_SB_OPT_SET_FD, &inval );
       ber_sockbuf_free( sb );

the last call causes a

       tcp_close( AC_SOCKET_INVALID )

which might be harmless provided it's intended. I've temporarily fixed it by adding a test in libraries/liblber/sockbuf.c

diff -u -r1.65 sockbuf.c
--- libraries/liblber/sockbuf.c 3 Jan 2006 22:12:07 -0000       1.65
+++ libraries/liblber/sockbuf.c 18 Aug 2006 09:34:34 -0000
@@ -553,8 +553,10 @@
       assert( sbiod != NULL );
       assert( SOCKBUF_VALID( sbiod->sbiod_sb ) );
-       tcp_close( sbiod->sbiod_sb->sb_fd );
-   return 0;
+       if ( sbiod->sbiod_sb->sb_fd != AC_SOCKET_INVALID ) {
+               tcp_close( sbiod->sbiod_sb->sb_fd );
+       }
+       return 0;

/* The argument is a pointer to the socket descriptor */

but might this hide other issues?

The same issue might appear later in connection_client_stop(), although I didn't experience it yet.


 -- Howard Chu
 Chief Architect, Symas Corp.  http://www.symas.com
 Director, Highland Sun        http://highlandsun.com/hyc
 OpenLDAP Core Team            http://www.openldap.org/project/