Deadlock with SIGALRM handling

[Michael Kelly <mike@weatherwax.co.uk>]

I've been using stress-ng to stress the VM subsystem:

# stress-ng -t4m --vm 32 --vm-bytes 750M --mmap 32 --mmap-bytes 750M 
--page-in

stress-ng forks a total of 128 extra stress-ng processes with this 
testcase and occasionally after the 4 minute timeout, when process 
termination occurs, a handful of processes do not die. The termination 
mechanism uses a combination of calls to alarm() and raising SIGALRM, 
SIGINT and SIGKILL signals. In summary there is a lot of signal raising 
and handling involving SIGALRM.

I found a deadlock situation involving 2 threads of a single process:

Thread 0: calls alarm() to set the itimer timeout via __setitimer 
(glibc/sysdeps/mach/hurd/setitimer.c). This locks _hurd_itimer_lock and 
then attempts to lock _hurd_siglock (within setitimer_locked) to check 
whether the global itimer signal preemptor has been registered.

Thread 1: is involved in handling a SIGALRM within post_signal 
(glibc/hurd/hurdsig.c). Just before it calls the global preemptors it 
locks _hurd_siglock. The itimer preemptor is then called which attempts 
to lock _hurd_itimer_lock.

I made a small alteration to unlock and relock the _hurd_itimer_lock 
when the _hurd_siglock is required within setitimer_locked. There is no 
state within the function that is invalidated by the unlock/lock so I 
think it it a suitable solution.

I've seen no incidence of this particular deadlock with this patch 
included. There is however an additional deadlock situation that this 
test-case generates involving syscall_task_terminate and _Xthread_abort 
which I am still working on.

I attach the patch for consideration.

Mike.

diff --git a/sysdeps/mach/hurd/setitimer.c b/sysdeps/mach/hurd/setitimer.c
index 95a21cb8..8f67308c 100644
--- a/sysdeps/mach/hurd/setitimer.c
+++ b/sysdeps/mach/hurd/setitimer.c
@@ -199,16 +199,20 @@ setitimer_locked (const struct itimerval *new, struct itimerval *old,
 	  __sigmask (SIGALRM), SI_TIMER, SI_TIMER,
 	  &restart_itimer,
 	};
-      if (!hurd_siglocked)
+      if (!hurd_siglocked) {
+	spin_unlock (&_hurd_itimer_lock);
 	__mutex_lock (&_hurd_siglock);
+      }
       if (! preemptor.next && _hurdsig_preemptors != &preemptor)
 	{
 	  preemptor.next = _hurdsig_preemptors;
 	  _hurdsig_preemptors = &preemptor;
 	  _hurdsig_preempted_set |= preemptor.signals;
 	}
-      if (!hurd_siglocked)
+      if (!hurd_siglocked) {
 	__mutex_unlock (&_hurd_siglock);
+	spin_lock (&_hurd_itimer_lock);
+      }
 
       if (_hurd_itimer_port == MACH_PORT_NULL)
 	{