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)
{