Attachment 109479 Details for Bug 86288 – [PATCH] os: Fix timer race conditions

[patch] [PATCH] os: Fix timer race conditions

0001-os-Fix-timer-race-conditions.patch (text/plain), 3.77 KB, created by Nikhil Mahale on 2014-11-14 18:24:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nikhil Mahale

Created: 2014-11-14 18:24:06 UTC

Size: 3.77 KB

patch

obsolete

>From 40f5100cb73aad8f90ba2c926dd08c4e15789de0 Mon Sep 17 00:00:00 2001
>From: Nikhil Mahale <nmahale@nvidia.com>
>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 <nmahale@nvidia.com>
>---
> os/WaitFor.c | 31 +++++++++++++++++++++----------
> 1 file changed, 21 insertions(+), 10 deletions(-)
>
>diff --git a/os/WaitFor.c b/os/WaitFor.c
>index 39fedd9..6e54222 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)
>@@ -408,10 +414,11 @@ DoTimer(OsTimerPtr timer, CARD32 now, OsTimerPtr *prev)
>     OsBlockSignals();
>     *prev = timer->next;
>     timer->next = NULL;
>+    OsReleaseSignals();
>+
>     newTime = (*timer->callback) (timer, now, timer->arg);
>     if (newTime)
>         TimerSet(timer, 0, newTime, timer->callback, timer->arg);
>-    OsReleaseSignals();
> }
> 
> OsTimerPtr
>@@ -515,8 +522,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
>

Actions: View | Splinter Review

Attachments on bug 86288: 109479

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.