[PATCH] Fix bug 11175 - Lots of winbindd zombie processes on Solaris platform

[Michael Adam]

Hi Jeremy,

the patch fixes the tests for me, it looks good.
I can push in a bit.

Just one question:

If I got it right, when sigaction is not available,
we don't support mutexes any more. Why not use the
old signal method in that case?

Michael

On 2015-03-25 at 14:06 -0700, Jeremy Allison wrote:
> On Wed, Mar 25, 2015 at 01:17:19PM +0100, Andreas Schneider wrote:
> > On Monday 23 March 2015 14:09:35 Jeremy Allison wrote:
> > > Fix confirmed by submitter. Changes use of signal()
> > > to sigaction().
> > 
> > Hi Jeremy,
> > 
> > I've reverted this patchset as it breaks 'make test' on most Linux platforms. 
> > Note that our autobuild doesn't have support for robust mutexes, so it doesn't 
> > fail ...
> > 
> > Sometimes I wonder if I'm the only one running 'make test' on his own boxes 
> > (Fedora and openSUSE).
> 
> Here is a working version that passes make test (sorry).
> 
> The problem was if a handler hadn't been installed already,
> then oldact.sa_handler == NULL (#define SIG_DFL	((__sighandler_t)0))
> which was returned and confused with the #else clause of
> #ifdef HAVE_SIGACTION (which returned NULL as guaranteed
> failure).
> 
> So we thought we should have working mutexes because
> tdb_mutex_locking_supported() would return true, but
> tdb_runtime_check_for_robust_mutexes() would always
> return false :-(.
> 
> New code returns a bool, and is given a pointer to
> fill with the returned handler.
> 
> Volker or Andreas, please review. We still need this
> (or something like it :-) for Solaris in 4.2.x.
> 
> Sorry for the original brown-paper-bag fix :-(.
> 
> Cheers,
> 
> 	Jeremy.

> From d8020f28806be85257cbdf2ce6f47f6b93dfe73c Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Fri, 20 Mar 2015 10:59:08 -0700
> Subject: [PATCH] lib: tdb: Use sigaction when testing for robust mutexes.
> 
> Working fix that copes with oldact.sa_handler == NULL
> if no handler initially set.
> 
> Fixes bug #11175 - Lots of winbindd zombie processes on Solaris platform.
> 
> https://bugzilla.samba.org/show_bug.cgi?id=11175
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  lib/tdb/common/mutex.c | 35 +++++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/tdb/common/mutex.c b/lib/tdb/common/mutex.c
> index 12f89d3..fae43d4 100644
> --- a/lib/tdb/common/mutex.c
> +++ b/lib/tdb/common/mutex.c
> @@ -713,6 +713,31 @@ cleanup_ma:
>  static void (*tdb_robust_mutext_old_handler)(int) = SIG_ERR;
>  static pid_t tdb_robust_mutex_pid = -1;
>  
> +static bool tdb_robust_mutex_setup_sigchild(void (*handler)(int),
> +			void (**p_old_handler)(int))
> +{
> +#ifdef HAVE_SIGACTION
> +	struct sigaction act;
> +	struct sigaction oldact;
> +
> +	memset(&act, '\0', sizeof(act));
> +
> +	act.sa_handler = handler;
> +#ifdef SA_RESTART
> +	act.sa_flags = SA_RESTART;
> +#endif
> +	sigemptyset(&act.sa_mask);
> +	sigaddset(&act.sa_mask, SIGCHLD);
> +	sigaction(SIGCHLD, &act, &oldact);
> +	if (p_old_handler) {
> +		*p_old_handler = oldact.sa_handler;
> +	}
> +	return true;
> +#else /* !HAVE_SIGACTION */
> +	return false;
> +#endif
> +}
> +
>  static void tdb_robust_mutex_handler(int sig)
>  {
>  	if (tdb_robust_mutex_pid != -1) {
> @@ -803,8 +828,10 @@ _PUBLIC_ bool tdb_runtime_check_for_robust_mutexes(void)
>  		goto cleanup_ma;
>  	}
>  
> -	tdb_robust_mutext_old_handler = signal(SIGCHLD,
> -					       tdb_robust_mutex_handler);
> +	if (tdb_robust_mutex_setup_sigchild(tdb_robust_mutex_handler,
> +			&tdb_robust_mutext_old_handler) == false) {
> +		goto cleanup_ma;
> +	}
>  
>  	tdb_robust_mutex_pid = fork();
>  	if (tdb_robust_mutex_pid == 0) {
> @@ -869,7 +896,7 @@ _PUBLIC_ bool tdb_runtime_check_for_robust_mutexes(void)
>  			goto cleanup_child;
>  		}
>  	}
> -	signal(SIGCHLD, tdb_robust_mutext_old_handler);
> +	tdb_robust_mutex_setup_sigchild(tdb_robust_mutext_old_handler, NULL);
>  
>  	ret = pthread_mutex_trylock(m);
>  	if (ret != EOWNERDEAD) {
> @@ -915,7 +942,7 @@ cleanup_child:
>  		}
>  	}
>  cleanup_sig_child:
> -	signal(SIGCHLD, tdb_robust_mutext_old_handler);
> +	tdb_robust_mutex_setup_sigchild(tdb_robust_mutext_old_handler, NULL);
>  cleanup_m:
>  	pthread_mutex_destroy(m);
>  cleanup_ma:
> -- 
> 2.2.0.rc0.207.ga3a616c
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150325/23e30ad9/attachment.pgp>