[Help-smalltalk] [PATCH] libgst: Fix SIGALRM race and cancel a timer before starting a new one

[Holger Hans Peter Freyther]

From: Holger Hans Peter Freyther <address@hidden>

The below Smalltalk code could trigger a race condition. We would program
a timer and then program another one before the first has expired. If the
original timer expires after we have set signal handler but before the
syscall to set the new timer was called. We could receive SIGALARM and
our signal handler would set SIG_DFL as the handler for SIGALARM. When
the new expiration is set and expires the default handler will terminate
the process.

Setting the application to SIG_IGN the handled signal could lead to a
deadlock as the real expiration is never signalled to the Smalltalk
side.

The best approach seems to cancel the timer. Before we have canceled
the timer we might run through the signal handler and signal the
original sempahore but that appears to be fine as we will not miss
an event or revert the signal handler too early. It might be best to
combine "TimeoutSem initialize" of Delay class>>#handleDelayRequstor
with the cancelation of the timer.

I have only verified the working of the posix timer (timer_settime)
and not the old interface.

Eval [
        | sem a |
        sem := Semaphore new.

        a := [
                [
                (Delay forMilliseconds: 250) wait.
                sem signal.
                ] repeat.
        ] fork.

        b := [
                [
                (Delay forMilliseconds: 125) timedWaitOn: sem.
                ] repeat.
        ] fork.

        c := [
                [
                (Delay forMilliseconds: 125) timedWaitOn: sem.
                ] repeat.
        ] fork.

        stdin next.
]
---
 libgst/ChangeLog             |  6 ++++++
 libgst/sysdep.h              |  4 ++++
 libgst/sysdep/posix/events.c |  1 +
 libgst/sysdep/posix/timer.c  | 19 +++++++++++++++++++
 4 files changed, 30 insertions(+)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index a390754..ce90b25 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,3 +1,9 @@
+2014-09-26  Holger Hans Peter Freyther  <address@hidden>
+
+       * sysdep.h: Declare _gst_sigalrm_cancel.
+       * sysdep/posix/events.c: Call _gst_sigalrm_cancel.
+       * sysdep/posix/timer.c: Implement _gst_sigalrm_cancel.
+
 2014-08-02  Holger Hans Peter Freyther  <address@hidden>
 
        * prims.def: Introduce VMpr_Behavior_newInitialize and
diff --git a/libgst/sysdep.h b/libgst/sysdep.h
index d872c32..5648ef2 100644
--- a/libgst/sysdep.h
+++ b/libgst/sysdep.h
@@ -108,6 +108,10 @@ extern void _gst_sigvtalrm_every (int deltaMilli,
                               SigHandler func)
   ATTRIBUTE_HIDDEN;
 
+/* Cancel an established SIGALRM */
+extern void _gst_sigalrm_cancel (void)
+  ATTRIBUTE_HIDDEN;
+
 /* Establish SIGALRM to be called when the nanosecond clock reaches NSTIME
    nanoseconds.  */
 extern void _gst_sigalrm_at (int64_t nsTime)
diff --git a/libgst/sysdep/posix/events.c b/libgst/sysdep/posix/events.c
index 2525b37..39d6ea9 100644
--- a/libgst/sysdep/posix/events.c
+++ b/libgst/sysdep/posix/events.c
@@ -200,6 +200,7 @@ void
 _gst_async_timed_wait (OOP semaphoreOOP,
                       int64_t milliTime)
 {
+  _gst_sigalrm_cancel ();
   _gst_async_interrupt_wait (semaphoreOOP, SIGALRM);
   _gst_sigalrm_at (milliTime);
 }
diff --git a/libgst/sysdep/posix/timer.c b/libgst/sysdep/posix/timer.c
index 9c8fe28..c47ae91 100644
--- a/libgst/sysdep/posix/timer.c
+++ b/libgst/sysdep/posix/timer.c
@@ -83,6 +83,25 @@ static mst_Boolean have_timer;
 #endif
 
 void
+_gst_sigalrm_cancel ()
+{
+#ifdef HAVE_TIMER_CREATE
+  if (have_timer)
+    {
+      struct itimerspec value;
+      memset(&value, 0, sizeof(value));
+      timer_settime (timer, TIMER_ABSTIME, &value, NULL);
+    }
+  else
+#endif
+    {
+      struct itimerval value;
+      memset(&value, 0, sizeof(value));
+      setitimer (ITIMER_REAL, &value, (struct itimerval *) 0);
+    }
+}
+
+void
 _gst_sigalrm_at (int64_t nsTime)
 {
 #ifdef HAVE_TIMER_CREATE
-- 
2.1.0