[lttng-dev] [PATCH userspace-rcu] use a filled signal mask to disable all signals

[Mathieu Desnoyers]

Hi Dave,

Nice catch ! I've reworked the patch to also fix compat_arch_x86.c and
use SIG_BLOCK at those sites (details in changelog). Here is the version
I plan to merge into userspace rcu. Thanks for spotting this!

Mathieu

commit 6ed4b2e64c6cef5d6d1a1c1943f29dbf4edf026a
Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Date:   Fri May 10 07:30:18 2013 -0400

    Fix: Use a filled signal mask to disable all signals
    
    Changelog from David Pelton's original patch:
    
    While using lttng-ust with an application that was calling fork()
    with pending signals, I found that all signals were getting unmasked
    shortly before the underlying call to fork().  After some
    investigation, I found that the rcu_bp_before_fork() function was
    unmasking all signals.  Based on the comments for this function, it
    should be masking all signals.  Inspection of the rest of the code
    in urcu-bp.c revealed the same pattern in two other functions.
    
    This patch changes the code to use a filled signal mask to disable
    all signals.  The change to rcu_bp_before_fork() addressed the
    problem I was seeing while using lttng-ust.  The changes to the
    other two functions appear to fix other instances of the same
    problem.
    
    Updates by Mathieu Desnoyers:
    
    - Use SIG_BLOCK instead of SIG_SETMASK when setting a filled mask. This
      has the same behavior in this case (since we're blocking all signals),
      but is semantically neater: if we ever some signals from that mask,
      we'd like to to a union with the signal mask already blocked by the
      application.
    - Also fix incorrect signal masking in compat_arch_x86.c.
    
    Reported-by: David Pelton <dpelton at ciena.com>
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>

diff --git a/compat_arch_x86.c b/compat_arch_x86.c
index 714201b..7d3b83a 100644
--- a/compat_arch_x86.c
+++ b/compat_arch_x86.c
@@ -80,9 +80,9 @@ static void mutex_lock_signal_save(pthread_mutex_t *mutex, sigset_t *oldmask)
 	int ret;
 
 	/* Disable signals */
-	ret = sigemptyset(&newmask);
+	ret = sigfillset(&newmask);
 	assert(!ret);
-	ret = pthread_sigmask(SIG_SETMASK, &newmask, oldmask);
+	ret = pthread_sigmask(SIG_BLOCK, &newmask, oldmask);
 	assert(!ret);
 	ret = pthread_mutex_lock(&compat_mutex);
 	assert(!ret);
diff --git a/urcu-bp.c b/urcu-bp.c
index ef1e687..a823659 100644
--- a/urcu-bp.c
+++ b/urcu-bp.c
@@ -213,9 +213,9 @@ void synchronize_rcu(void)
 	sigset_t newmask, oldmask;
 	int ret;
 
-	ret = sigemptyset(&newmask);
+	ret = sigfillset(&newmask);
 	assert(!ret);
-	ret = pthread_sigmask(SIG_SETMASK, &newmask, &oldmask);
+	ret = pthread_sigmask(SIG_BLOCK, &newmask, &oldmask);
 	assert(!ret);
 
 	mutex_lock(&rcu_gp_lock);
@@ -385,9 +385,9 @@ void rcu_bp_register(void)
 	sigset_t newmask, oldmask;
 	int ret;
 
-	ret = sigemptyset(&newmask);
+	ret = sigfillset(&newmask);
 	assert(!ret);
-	ret = pthread_sigmask(SIG_SETMASK, &newmask, &oldmask);
+	ret = pthread_sigmask(SIG_BLOCK, &newmask, &oldmask);
 	assert(!ret);
 
 	/*
@@ -420,9 +420,9 @@ void rcu_bp_before_fork(void)
 	sigset_t newmask, oldmask;
 	int ret;
 
-	ret = sigemptyset(&newmask);
+	ret = sigfillset(&newmask);
 	assert(!ret);
-	ret = pthread_sigmask(SIG_SETMASK, &newmask, &oldmask);
+	ret = pthread_sigmask(SIG_BLOCK, &newmask, &oldmask);
 	assert(!ret);
 	mutex_lock(&rcu_gp_lock);
 	saved_fork_signal_mask = oldmask;

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com