2.5.39: Signal delivery to thread groups: Bug or feature

2.5.39: Signal delivery to thread groups: Bug or feature

[Axel]

Hello,
I played a little bit with the new clone flags and wrote a small test
program using two threads: The first (initial) thread blocks all signals. The
second thread is created with all signals blocked and inherits the signal
mask of the initial thread. It unblocks SIGINT and calls sys_rt_sigtimedwait
with the remaining signal mask. Therefore it waits for all signals with
exception of SIGINT. In the kernel this yields to an empty signal mask for
this thread during the sigwait. No signal handler is installed by the
process. Now an external SIGINT is delivered to the whole process: The
signal delivery code decides to send this signal directly to the initial
 thread because no user handler is installed and the signal mask for this
 thread blocks the signal. The second thread never receives the SIGINT.

Reason:
The main signal dispatching function send_sig_info in kernel/signal.c
requires at the moment an installed signal handler for delivery of signals
to members of the thread group.

Therefore NPTL/NPT must install signal handlers for all signals
during startup to allow signal delivery to other threads and must restore
these default handlers to SIG_DFL after first delivery and raise
the signal to create the correct exit code.
IMHO, the current signal system is not a clean solution (yet), but of course
much better than the ugly signal forwarding required by the thread group
leader only working as signal thread in 2.4.X.

Axel

Re: 2.5.39: Signal delivery to thread groups: Bug or feature

[Axel]

[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]

Hello Ingo,
On Sunday 29 September 2002 10:25, you wrote:
> On Sat, 28 Sep 2002, Axel wrote:
> > I played a little bit with the new clone flags and wrote a small test
> > program using two threads: The first (initial) thread blocks all
> > signals. The second thread is created with all signals blocked and
> > inherits the signal mask of the initial thread. It unblocks SIGINT and
> > calls sys_rt_sigtimedwait with the remaining signal mask. Therefore it
> > waits for all signals with exception of SIGINT. In the kernel this
> > yields to an empty signal mask for this thread during the sigwait. No
> > signal handler is installed by the process. Now an external SIGINT is
> > delivered to the whole process: The signal delivery code decides to send
> > this signal directly to the initial thread because no user handler is
> > installed and the signal mask for this thread blocks the signal. The
> > second thread never receives the SIGINT.
>
> could you send me the testcase? Thanks,
unfortunately, my test case is part of a thread library which was intended as
replacement for the old linuxthreads library. The idea of this library is to 
have a two level thread library, i.e. posix threads with M:N scheduling on 
top of posix threads with 1:1 scheduling. Starting with a 2.4.18+NGPT patches 
(futexes+tkill) kernel, I implemented a user level signal forwarding scheme, 
for the kernel threads, which worked as expected - slow and with a lot of 
system calls. 
The test program is attached - in principle a test case from NGPT (change 
kth* to pthread*) and use NPT(L) as underlying library.  

After having a look at your changes starting with 2.5.35? i decided to drop 
further development for the old signal scheme and converted the library to 
use all the advantages of the 2.5.X kernels - some of the test cases stopped 
working and I had to look for the reasons. 

I will test your changes to the kernel as soon as possible
IMHO, they will not work as expected, because in the function 
find_unblocked_thread() the real_blocked mask of the thread is also checked:
a thread with all signals blocked calls sys_rt_sigtimedwait to wait for all 
signals, all other threads block all signals. If no signal is pending, the 
real_blocked mask of this thread is set to all filled and the blocked mask
of this thread is set to empty. Later a signal is sent to the process and the 
find_blocked_thread function detects that the sigwait thread has this signal 
not set in its blocked mask but set in its real_blocked mask and does not 
deliver the signal to this thread as it should.
BTW, what is the reason for the existance of the real_blocked mask? I found a 
usage of it only during the sigwaits to store the original signal mask. May 
be a local variable would be a cleaner solution.

Axel

Please CC all mails to me, because I read only the archives of the 
linux-kernel mailing list.





[-- Attachment #2: Sigwait test program, --]
[-- Type: text/x-c, Size: 2058 bytes --]

#include <kth.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <signal.h>
#include <string.h>
#include <sys/types.h>

void *catch_usr1(void *);

void *catch_usr1(void *p)
{
	int signo = SIGUSR1;
	int caught;
	sigset_t sigs_to_catch;
	sigset_t s;
	/* Unblock SIGINT */
	sigemptyset(&sigs_to_catch);
	sigaddset(&sigs_to_catch, SIGINT);
	kth_sigmask(SIG_UNBLOCK, &sigs_to_catch, &s);
	/* Identify our thread */
	printf("catch_usr1: signal %d processing running as thread %lu \n",
	       signo, kth_self());
	printf("catch_usr1: Someone please send pid %d a SIGUSR1\n", getpid());

	/*
	 * We inherited a thread sigmask with all the signals 
	 * blocked.  So, we can wait on whatever signals we're
	 * interested in and (as long as no other thread waits
	 * for them) we'll be sure return from sigwait() to
	 * handle it.
	 */

	/* set this thread's signal mask to block out all other signals */
	sigaddset(&sigs_to_catch, signo);
	sigwait(&sigs_to_catch, &caught);

	printf("catch_usr1: signal %d processing thread caught signal %d\n",
	       signo, caught);
	return (p);
}

extern int main(void)
{
	int i;
	kth_t threads[1];
	int num_threads = 0;
	sigset_t sigs_to_block;

	/* Identify our thread */
	printf("main: running in thread 0x%x\n", (int)kth_self());
	/* 
	 * Set this thread's signal mask to block out all other signals
	 * Other thread's will inherit the mask
	 */
	/* BLOCK ALL SIGNALS */
	sigfillset(&sigs_to_block);
	/* sigdelset(&sigs_to_block,SIGINT); */
	kth_sigmask(SIG_BLOCK, &sigs_to_block, NULL);
	/* Set signal handler for catching SIGSEGV and SIGBUS */

	/* Rather than install the action/handler for the process,
	   we create a thread to wait for the signal */
	kth_create(&threads[num_threads++], NULL, catch_usr1, NULL);
	printf("main: %d threads created\n", num_threads);
	/* wait until all threads have finished */
	for (i = 0; i < num_threads; i++) {
		kth_join(threads[i], NULL);
		printf("main: joined to thread %d \n", i);
	}
	printf("main: all %d threads have finished. \n", num_threads);
	return 0;
}

Re: 2.5.39: Signal delivery to thread groups: Bug or feature

[Roland McGrath]

> we still have one more problem left in the signal handling area: atomicity
> of signal delivery. Eg. right now it's possible to have a signal 'in
> flight' for one specific thread, which manages to block it before handling
> the signal. What should the behavior be in that case? Does POSIX say
> anything about this?

Assuming you are talking about a process-global signal (not pthread_kill),
then POSIX does not permit this race condition.  If there is any thread
that can take the signal (i.e. not blocking it, or is sigwait'ing for it),
then one such thread must take the signal.  The selection of the thread and
it beginning its action (i.e. choosing a signal handler, and saving the
signal mask the signal handler will restore on its return; or process death)
must be atomic with respect to that thread blocking the signal.

Re: 2.5.39: Signal delivery to thread groups: Bug or feature

[Ingo Molnar]

On Sat, 28 Sep 2002, Axel wrote:

> The main signal dispatching function send_sig_info in kernel/signal.c
> requires at the moment an installed signal handler for delivery of
> signals to members of the thread group.

i fixed this bug, see the attached attached patch, it's against BK-curr.  

It fixes the testcase Ulrich created from your description - does it fix
your testcase as well? (Ulrich added this testcase to NPTL, should show up
in the next drop.)

we still have one more problem left in the signal handling area: atomicity
of signal delivery. Eg. right now it's possible to have a signal 'in
flight' for one specific thread, which manages to block it before handling
the signal. What should the behavior be in that case? Does POSIX say
anything about this?

	Ingo

--- linux/kernel/signal.c.orig	Sun Sep 29 11:59:03 2002
+++ linux/kernel/signal.c	Sun Sep 29 12:17:14 2002
@@ -874,9 +874,23 @@
 	return err;
 }
 
+struct task_struct * find_unblocked_thread(struct task_struct *p, int signr)
+{
+	struct task_struct *tmp;
+	struct list_head *l;
+	struct pid *pid;
+
+	for_each_task_pid(p->tgid, PIDTYPE_TGID, tmp, l, pid)
+		if (!sigismember(&tmp->blocked, signr) &&
+					!sigismember(&tmp->real_blocked, signr))
+			return tmp;
+	return NULL;
+}
+
 int
 send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
 {
+	struct task_struct *t;
 	unsigned long flags;
 	int ret = 0;
 
@@ -905,21 +919,19 @@
 	if (sig_ignored(p, sig))
 		goto out_unlock;
 
-	/* blocked (or ptraced) signals get posted */
-	spin_lock(&p->sigmask_lock);
-	if ((p->ptrace & PT_PTRACED) || sigismember(&p->blocked, sig) ||
-					sigismember(&p->real_blocked, sig)) {
-		spin_unlock(&p->sigmask_lock);
+	if (sig_kernel_specific(sig))
 		goto out_send;
-	}
-	spin_unlock(&p->sigmask_lock);
 
+	/* Does any of the threads unblock the signal? */
+	t = find_unblocked_thread(p, sig);
+	if (!t) {
+		ret = __send_sig_info(sig, info, p, 1);
+		goto out_unlock;
+	}
 	if (sig_kernel_broadcast(sig) || sig_kernel_coredump(sig)) {
 		ret = __broadcast_thread_group(p, sig);
 		goto out_unlock;
 	}
-	if (sig_kernel_specific(sig))
-		goto out_send;
 
 	/* must not happen */
 	BUG();

Re: 2.5.39: Signal delivery to thread groups: Bug or feature

[Ingo Molnar]

On Sat, 28 Sep 2002, Axel wrote:

> I played a little bit with the new clone flags and wrote a small test
> program using two threads: The first (initial) thread blocks all
> signals. The second thread is created with all signals blocked and
> inherits the signal mask of the initial thread. It unblocks SIGINT and
> calls sys_rt_sigtimedwait with the remaining signal mask. Therefore it
> waits for all signals with exception of SIGINT. In the kernel this
> yields to an empty signal mask for this thread during the sigwait. No
> signal handler is installed by the process. Now an external SIGINT is
> delivered to the whole process: The signal delivery code decides to send
> this signal directly to the initial thread because no user handler is
> installed and the signal mask for this thread blocks the signal. The
> second thread never receives the SIGINT.

could you send me the testcase? Thanks,

	Ingo

2.5.39: Signal delivery to thread groups: Bug or feature

[Axel]

Hello, 
I played a little bit with the new clone flags and wrote a small test 
program using two threads: The first (initial) thread blocks all signals. The 
second thread is created with all signals blocked and inherits the signal 
mask of the initial thread. It unblocks SIGINT and calls sys_rt_sigtimedwait 
with the remaining signal mask. Therefore it waits for all signals with 
exception of SIGINT. In the kernel this yields to an empty signal mask for 
this thread during the sigwait. No signal handler is installed by the 
process. Now an external SIGINT is delivered to the whole process: The 
signal delivery code decides to send this signal directly to the initial thread 
because no user handler is installed and the signal mask for this thread 
blocks the signal. The second thread never receives the SIGINT.

Reason:
The main signal dispatching function send_sig_info in kernel/signal.c 
requires at the moment an installed signal handler for delivery of signals 
to members of the thread group.

Therefore NPTL/NPT must install signal handlers for all signals 
during startup to allow signal delivery to other threads and must restore
these default handlers to SIG_DFL after first delivery and raise
the signal to create the correct exit code.
IMHO, the current signal system is not a clean solution (yet), but of course
much better than the ugly signal forwarding required by the thread group 
leader only working as signal thread in 2.4.X. 

Axel