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

Re: kqueue in OpenLDAP for FreeBSD (fwd)

Looks like there's a little bit more work necessary for kqueue support to function correctly. 

--Quanah

------------ Forwarded Message ------------
Date: Monday, October 23, 2017 12:43 AM -0700
From: Xin Li <delphij@delphij.net>
To: Quanah Gibson-Mount <quanah@symas.com>, Xin LI <delphij@gmail.com>
Cc: d@delphij.net, Pietro Cerutti <gahr@gahr.ch>, Xin LI <delphij@freebsd.org> 
Subject: Re: kqueue in OpenLDAP for FreeBSD

Hi, Quanah,

I finally got some time to twiddle with OpenLDAP (again).  It looks like
the EBADF is expected, because fork(2) says:

          •   The child process has its own copy of the parent's
              descriptors, except for descriptors returned by
              kqueue(2), which are not inherited from the parent
              process.

And slapd does have a fork() after the initial kqueue(), which rendered
it invalid.

The kqueue() is part of SLAP_SOCK_INIT(), which is part of
slapd_daemon_init().  Then, after that, fork() happen in lutil_detach(),
and doing this hack would (incorrectly) make slapd to start:

diff --git a/libraries/liblutil/detach.c b/libraries/liblutil/detach.c
index c6464c038..f47d36548 100644
--- a/libraries/liblutil/detach.c
+++ b/libraries/liblutil/detach.c
@@ -73,7 +73,7 @@ lutil_detach( int debug, int do_close )
#ifdef HAVE_THR
                       pid = fork1();
#else
-                       pid = fork();
+                       pid = rfork(RFPROC);
#endif
                       switch ( pid )
                       {

It is incorrect because the code in slapd/main.c seems to expect the
child to write a "1" to it before exiting with EXIT_SUCCESS() and
obviously if the two processes shares the same file table, the parent
would consider the start was failed because read() would fail.

I think the right fix would be to move the lutil_detach() to before
slapd_daemon_init(), see attachment, but it seems that some code after
the initial daemon initialization is still trying to output to stderr, etc.

What do you think?

Cheers,
---------- End Forwarded Message ----------



--

Quanah Gibson-Mount
Product Architect
Symas Corporation
Packaged, certified, and supported LDAP solutions powered by OpenLDAP:
<http://www.symas.com>
--- Begin Message --- 
Hi, Quanah,

I finally got some time to twiddle with OpenLDAP (again).  It looks like
the EBADF is expected, because fork(2) says:

           •   The child process has its own copy of the parent's
               descriptors, except for descriptors returned by
               kqueue(2), which are not inherited from the parent
               process.

And slapd does have a fork() after the initial kqueue(), which rendered
it invalid.

The kqueue() is part of SLAP_SOCK_INIT(), which is part of
slapd_daemon_init().  Then, after that, fork() happen in lutil_detach(),
and doing this hack would (incorrectly) make slapd to start:

diff --git a/libraries/liblutil/detach.c b/libraries/liblutil/detach.c
index c6464c038..f47d36548 100644
--- a/libraries/liblutil/detach.c
+++ b/libraries/liblutil/detach.c
@@ -73,7 +73,7 @@ lutil_detach( int debug, int do_close )
 #ifdef HAVE_THR
                        pid = fork1();
 #else
-                       pid = fork();
+                       pid = rfork(RFPROC);
 #endif
                        switch ( pid )
                        {

It is incorrect because the code in slapd/main.c seems to expect the
child to write a "1" to it before exiting with EXIT_SUCCESS() and
obviously if the two processes shares the same file table, the parent
would consider the start was failed because read() would fail.

I think the right fix would be to move the lutil_detach() to before
slapd_daemon_init(), see attachment, but it seems that some code after
the initial daemon initialization is still trying to output to stderr, etc.

What do you think?

Cheers,

On 10/20/17 09:00, Quanah Gibson-Mount wrote:
> --On Friday, October 20, 2017 8:15 AM -0700 Quanah Gibson-Mount
> <quanah@symas.com> wrote:
> 
>> Thanks, I'll play around with the build some.  I'm limiting slapd to as
>> minimal a build as possible with my configure:
>>
>> ./configure --without-tls --without-cyrus-sasl --enable-ldap
>> --enable-meta --enable-accesslog --enable-ppolicy --enable-rewrite
>> --enable-rwm --prefix=/tmp/q
> 
> 
> I've updated my configure to:
> 
> CFLAGS="-O2 -pipe  -fstack-protector -DLDAP_DEPRECATED
> -fno-strict-aliasing" \
> ./configure \
> --with-threads=posix  --with-tls=openssl --disable-dependency-tracking \
> --enable-dynamic --without-cyrus-sasl \
> --localstatedir=/var/db  --enable-crypt  --enable-ldap \
> --enable-meta  --enable-rewrite  --enable-null \
> --enable-monitor --disable-seqmod --enable-syncprov \
> --enable-mdb --without-fetch \
> --prefix=/tmp/q
> 
> 
> And I still have no issues.  I'm unable to get the freebsd version of
> libtool to build shared libraries, so I can't test dynamic modules.
> 
> --Quanah
> 
> 
> -- 
> 
> Quanah Gibson-Mount
> Product Architect
> Symas Corporation
> Packaged, certified, and supported LDAP solutions powered by OpenLDAP:
> <http://www.symas.com>
> 

diff --git a/servers/slapd/main.c b/servers/slapd/main.c
index 0a053c3ee..18d840c5a 100644
--- a/servers/slapd/main.c
+++ b/servers/slapd/main.c
@@ -692,6 +692,9 @@ unhandled_option:;
 	ber_set_option(NULL, LBER_OPT_DEBUG_LEVEL, &slap_debug);
 	ldap_set_option(NULL, LDAP_OPT_DEBUG_LEVEL, &slap_debug);
 	ldif_debug = slap_debug;
+	if ( check != CHECK_NONE ) {
+		no_detach = 1;
+	}
 
 	if ( version ) {
 		fprintf( stderr, "%s\n", Versionstr );
@@ -739,6 +742,29 @@ unhandled_option:;
 	global_host = ldap_pvt_get_fqdn( NULL );
 	ber_str2bv( global_host, 0, 0, &global_host_bv );
 
+#ifndef HAVE_WINSOCK
+	if ( !no_detach ) {
+		if ( lutil_pair( waitfds ) < 0 ) {
+			Debug( LDAP_DEBUG_ANY,
+				"main: lutil_pair failed: %d\n",
+				0, 0, 0 );
+			rc = 1;
+			goto destroy;
+		}
+		pid = lutil_detach( no_detach, 0 );
+		if ( pid ) {
+			char buf[4];
+			rc = EXIT_SUCCESS;
+			close( waitfds[1] );
+			if ( read( waitfds[0], buf, 1 ) != 1 )
+				rc = EXIT_FAILURE;
+			_exit( rc );
+		} else {
+			close( waitfds[0] );
+		}
+	}
+#endif /* HAVE_WINSOCK */
+
 	if( check == CHECK_NONE && slapd_daemon_init( urls ) != 0 ) {
 		rc = 1;
 		SERVICE_EXIT( ERROR_SERVICE_SPECIFIC_ERROR, 16 );
@@ -911,29 +937,6 @@ unhandled_option:;
 	(void) SIGNAL( SIGBREAK, slap_sig_shutdown );
 #endif
 
-#ifndef HAVE_WINSOCK
-	if ( !no_detach ) {
-		if ( lutil_pair( waitfds ) < 0 ) {
-			Debug( LDAP_DEBUG_ANY,
-				"main: lutil_pair failed: %d\n",
-				0, 0, 0 );
-			rc = 1;
-			goto destroy;
-		}
-		pid = lutil_detach( no_detach, 0 );
-		if ( pid ) {
-			char buf[4];
-			rc = EXIT_SUCCESS;
-			close( waitfds[1] );
-			if ( read( waitfds[0], buf, 1 ) != 1 )
-				rc = EXIT_FAILURE;
-			_exit( rc );
-		} else {
-			close( waitfds[0] );
-		}
-	}
-#endif /* HAVE_WINSOCK */
-
 #ifdef CSRIMALLOC
 	mal_leaktrace(1);
 #endif

Attachment: signature.asc
Description: OpenPGP digital signature


--- End Message ---