From: Holger Hans Peter Freyther <[hidden email]>
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 <[hidden email]> + + * 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 <[hidden email]> * 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 _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
Il 26/09/2014 16:21, Holger Hans Peter Freyther ha scritto:
> From: Holger Hans Peter Freyther <[hidden email]> > > 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. Thanks, this is okay. It is racy anyway whether the previously-registered timer would happen anyway; canceling the timer solves the race in favor of the new timer. Paolo > 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 <[hidden email]> > + > + * 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 <[hidden email]> > > * 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 > Paolo _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
On Fri, Sep 26, 2014 at 04:52:41PM +0200, Paolo Bonzini wrote:
> Thanks, this is okay. It is racy anyway whether the > previously-registered timer would happen anyway; canceling the timer > solves the race in favor of the new timer. do you know that posix make any guarantee that either the signal of the previous timer has been delivered or the old timer has been canceled? For clock monotinic I did find common_timer_set inide the kernel and it seems that it tries to prevent such thing? _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
Il 26/09/2014 20:26, Holger Hans Peter Freyther ha scritto:
> do you know that posix make any guarantee that either the signal of > the previous timer has been delivered or the old timer has been > canceled? Yes, definitely. The old timer must be delivered before timer_settime/setitimer returns. Paolo > For clock monotinic I did find common_timer_set inide the kernel > and it seems that it tries to prevent such thing? > > > _______________________________________________ help-smalltalk mailing list [hidden email] https://lists.gnu.org/mailman/listinfo/help-smalltalk |
Free forum by Nabble | Edit this page |