Deadlock with SIGALRM handling

Michael Kelly <[email protected]>
Newsgroups gmane.linux.debian.ports.hurd
Message-ID <[email protected]>
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.
setitimer.c.patch (text/x-patch, 871 B)
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)
 	{
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.