On Monday, November 24, 2014 10:52:37 PM Julien Cristau wrote: > On Fri, Nov 14, 2014 at 23:52:55 +0530, Nikhil Mahale wrote: > > From 40f5100cb73aad8f90ba2c926dd08c4e15789de0 Mon Sep 17 00:00:00 2001 > > From: Nikhil Mahale <[email protected]> > > Date: Tue, 11 Nov 2014 17:44:28 +0530 > > Subject: [PATCH] os: Fix timer race conditions > > > > Fixing following kind of race-conditions - > > > > WaitForSomething() > > > > ----> // timers -> timer-1 -> timer-2 -> null > > > > while (timers && (int) (timers->expires - now) <= 0) > > > > // prototype - DoTimer(OsTimerPtr timer, CARD32 now, > > OsTimerPtr *prev) > > DoTimer(timers, now, &timers) > > > > > > ----> OsBlockSignals(); .... OS Signal comes just before > > blocking it, > > > > .... timer-1 handler gets called. > > > > // timer-1 gets served and > > scheduled again; > > // timers -> timer-2 -> > > timer-1 -> null > > > > .... > > > > *prev = timer->next; > > > > timer->next = NULL; // timers -> null > > // timers list gets corrupted here and timer-2 gets > > removed from list. > > > > https://bugs.freedesktop.org/show_bug.cgi?id=86288 > > Signed-off-by: Nikhil Mahale <[email protected]> > > --- > > > > os/WaitFor.c | 31 +++++++++++++++++++++---------- > > 1 file changed, 21 insertions(+), 10 deletions(-) > > As far as I can tell after this change DoTimer is always called with > signals disabled, so it doesn't need to disable them itself? >
ok, here is updated patch. > Cheers, > Julien ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
>From 5c797ce9cdb596107f9ebcdf8345ca234c587b22 Mon Sep 17 00:00:00 2001 From: Nikhil Mahale <[email protected]> Date: Tue, 11 Nov 2014 17:44:28 +0530 Subject: [PATCH] os: Fix timer race conditions Fixing following kind of race-conditions - WaitForSomething() | ----> // timers -> timer-1 -> timer-2 -> null while (timers && (int) (timers->expires - now) <= 0) // prototype - DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev) DoTimer(timers, now, &timers) | | ----> OsBlockSignals(); .... OS Signal comes just before blocking it, .... timer-1 handler gets called. // timer-1 gets served and scheduled again; // timers -> timer-2 -> timer-1 -> null .... *prev = timer->next; timer->next = NULL; // timers -> null // timers list gets corrupted here and timer-2 gets removed from list. https://bugs.freedesktop.org/show_bug.cgi?id=86288 Signed-off-by: Nikhil Mahale <[email protected]> --- os/WaitFor.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/os/WaitFor.c b/os/WaitFor.c index 39fedd9..e768356 100644 --- a/os/WaitFor.c +++ b/os/WaitFor.c @@ -123,7 +123,7 @@ struct _OsTimerRec { static void DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev); static void CheckAllTimers(void); -static OsTimerPtr timers = NULL; +static volatile OsTimerPtr timers = NULL; /***************** * WaitForSomething: @@ -263,11 +263,14 @@ WaitForSomething(int *pClientsReady) if ((int) (timers->expires - now) <= 0) expired = 1; - while (timers && (int) (timers->expires - now) <= 0) - DoTimer(timers, now, &timers); + if (expired) { + OsBlockSignals(); + while (timers && (int) (timers->expires - now) <= 0) + DoTimer(timers, now, &timers); + OsReleaseSignals(); - if (expired) return 0; + } } } else { @@ -281,11 +284,14 @@ WaitForSomething(int *pClientsReady) if ((int) (timers->expires - now) <= 0) expired = 1; - while (timers && (int) (timers->expires - now) <= 0) - DoTimer(timers, now, &timers); + if (expired) { + OsBlockSignals(); + while (timers && (int) (timers->expires - now) <= 0) + DoTimer(timers, now, &timers); + OsReleaseSignals(); - if (expired) return 0; + } } } if (someReady) @@ -405,13 +411,12 @@ DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev) { CARD32 newTime; - OsBlockSignals(); *prev = timer->next; timer->next = NULL; + newTime = (*timer->callback) (timer, now, timer->arg); if (newTime) TimerSet(timer, 0, newTime, timer->callback, timer->arg); - OsReleaseSignals(); } OsTimerPtr @@ -515,8 +520,12 @@ TimerCheck(void) { CARD32 now = GetTimeInMillis(); - while (timers && (int) (timers->expires - now) <= 0) - DoTimer(timers, now, &timers); + if (timers && (int) (timers->expires - now) <= 0) { + OsBlockSignals(); + while (timers && (int) (timers->expires - now) <= 0) + DoTimer(timers, now, &timers); + OsReleaseSignals(); + } } void -- 1.8.1.5
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
