frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=86288
Signed-off-by: Nikhil Mahale <nmahale@nvidia.com>
Reviewed-by: Julien Cristau <jcristau@debian.org>

v2: Apply warning fixes from Keith Packard <keithp@keithp.com>

Reviewed-by: Aaron Plattner <aplattner@nvidia.com>
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
This commit is contained in:
Nikhil Mahale 2015-01-24 17:06:59 -08:00 committed by Keith Packard
parent 58f28b0427
commit fe4c774c57
1 changed files with 26 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
os/WaitFor.c
View File

@ -121,9 +121,9 @@ struct _OsTimerRec {
void *arg; void *arg;
}; };
static void DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev); static void DoTimer(OsTimerPtr timer, CARD32 now, volatile OsTimerPtr *prev);
static void CheckAllTimers(void); static void CheckAllTimers(void);
static OsTimerPtr timers = NULL; static volatile OsTimerPtr timers = NULL;
/***************** /*****************
* WaitForSomething: * WaitForSomething:
@ -263,13 +263,16 @@ WaitForSomething(int *pClientsReady)
if ((int) (timers->expires - now) <= 0) if ((int) (timers->expires - now) <= 0)
expired = 1; expired = 1;
if (expired) {
OsBlockSignals();
while (timers && (int) (timers->expires - now) <= 0) while (timers && (int) (timers->expires - now) <= 0)
DoTimer(timers, now, &timers); DoTimer(timers, now, &timers);
OsReleaseSignals();
if (expired)
return 0; return 0;
} }
} }
}
else { else {
fd_set tmp_set; fd_set tmp_set;
@ -281,13 +284,16 @@ WaitForSomething(int *pClientsReady)
if ((int) (timers->expires - now) <= 0) if ((int) (timers->expires - now) <= 0)
expired = 1; expired = 1;
if (expired) {
OsBlockSignals();
while (timers && (int) (timers->expires - now) <= 0) while (timers && (int) (timers->expires - now) <= 0)
DoTimer(timers, now, &timers); DoTimer(timers, now, &timers);
OsReleaseSignals();
if (expired)
return 0; return 0;
} }
} }
}
if (someReady) if (someReady)
XFD_ORSET(&LastSelectMask, &ClientsWithInput, &LastSelectMask); XFD_ORSET(&LastSelectMask, &ClientsWithInput, &LastSelectMask);
if (AnyClientsWriteBlocked && XFD_ANYSET(&clientsWritable)) { if (AnyClientsWriteBlocked && XFD_ANYSET(&clientsWritable)) {
@ -401,24 +407,25 @@ CheckAllTimers(void)
} }
static void static void
DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev) DoTimer(OsTimerPtr timer, CARD32 now, volatile OsTimerPtr *prev)
{ {
CARD32 newTime; CARD32 newTime;
OsBlockSignals(); OsBlockSignals();
*prev = timer->next; *prev = timer->next;
timer->next = NULL; timer->next = NULL;
OsReleaseSignals();
newTime = (*timer->callback) (timer, now, timer->arg); newTime = (*timer->callback) (timer, now, timer->arg);
if (newTime) if (newTime)
TimerSet(timer, 0, newTime, timer->callback, timer->arg); TimerSet(timer, 0, newTime, timer->callback, timer->arg);
OsReleaseSignals();
} }
OsTimerPtr OsTimerPtr
TimerSet(OsTimerPtr timer, int flags, CARD32 millis, TimerSet(OsTimerPtr timer, int flags, CARD32 millis,
OsTimerCallback func, void *arg) OsTimerCallback func, void *arg)
{ {
register OsTimerPtr *prev; volatile OsTimerPtr *prev;
CARD32 now = GetTimeInMillis(); CARD32 now = GetTimeInMillis();
if (!timer) { if (!timer) {
@ -470,7 +477,7 @@ Bool
TimerForce(OsTimerPtr timer) TimerForce(OsTimerPtr timer)
{ {
int rc = FALSE; int rc = FALSE;
OsTimerPtr *prev; volatile OsTimerPtr *prev;
OsBlockSignals(); OsBlockSignals();
for (prev = &timers; *prev; prev = &(*prev)->next) { for (prev = &timers; *prev; prev = &(*prev)->next) {
@ -487,7 +494,7 @@ TimerForce(OsTimerPtr timer)
void void
TimerCancel(OsTimerPtr timer) TimerCancel(OsTimerPtr timer)
{ {
OsTimerPtr *prev; volatile OsTimerPtr *prev;
if (!timer) if (!timer)
return; return;
@ -515,8 +522,12 @@ TimerCheck(void)
{ {
CARD32 now = GetTimeInMillis(); CARD32 now = GetTimeInMillis();
if (timers && (int) (timers->expires - now) <= 0) {
OsBlockSignals();
while (timers && (int) (timers->expires - now) <= 0) while (timers && (int) (timers->expires - now) <= 0)
DoTimer(timers, now, &timers); DoTimer(timers, now, &timers);
OsReleaseSignals();
}
} }
void void