[8/11] kernel32: Implement the timer queue thread.

Dan Hipschman dsh at linux.ucla.edu
Fri Jul 18 19:19:11 CDT 2008


This patch implements the timer thread, running callbacks, cleaning up
timers, and all that good stuff.  It should be very efficient, using mostly
atomic operations and one critical section for thread safety.  I designed
it with thread-safety in mind, and once I got it working the way I wanted,
ran the tests in a loop a few thousand times while I ate lunch.  I also did
this with the following patches and never once got a single error, hang or
crash.

It punts on some integer-wrapping issues.  If someone runs their wineserver
for 49 days they may run into that issue.  I just thought it would distract
from the main content and wanted to finish everything else before I thought
about it.

Note that the one todo_wine that got added in the tests wasn't because
something broke, it's just that the function never ran before.

---
 dlls/kernel32/sync.c       |  214 ++++++++++++++++++++++++++++++++++++++++----
 dlls/kernel32/tests/sync.c |   42 +++++++--
 2 files changed, 231 insertions(+), 25 deletions(-)

diff --git a/dlls/kernel32/sync.c b/dlls/kernel32/sync.c
index 8085077..4df5b44 100644
--- a/dlls/kernel32/sync.c
+++ b/dlls/kernel32/sync.c
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "wine/port.h"
 
+#include <assert.h>
 #include <string.h>
 #ifdef HAVE_UNISTD_H
 # include <unistd.h>
@@ -1043,17 +1044,145 @@ BOOL WINAPI CancelWaitableTimer( HANDLE handle )
 }
 
 
+struct timer_queue;
 struct queue_timer
 {
+    struct timer_queue *q;
     struct list entry;
+    LONG runcount;              /* number of callbacks pending execution */
+    WAITORTIMERCALLBACK callback;
+    PVOID param;
+    DWORD period;
+    ULONG flags;
+    DWORD expire;
+    BOOL die;          /* timer should be deleted; once set, never unset */
 };
 
 struct timer_queue
 {
     RTL_CRITICAL_SECTION cs;
     struct list timers;
+    struct queue_timer *next;   /* the next timer to expire */
+    BOOL quit;         /* queue should be deleted; once set, never unset */
+    DWORD timeout;              /* when the next timer expires */
+    HANDLE event;
+    HANDLE thread;
 };
 
+static void queue_remove_timer(struct timer_queue *q, struct queue_timer *t)
+{
+    /* We MUST hold the queue cs while calling this function.  This ensures
+       that we cannot queue another callback for this timer.  The runcount
+       being zero makes sure we don't have any already queued.  */
+    assert(t->runcount == 0);
+    assert(t->die);
+    list_remove(&t->entry);
+    if (q->next == t)
+        q->next = NULL;
+    HeapFree(GetProcessHeap(), 0, t);
+}
+
+static DWORD WINAPI timer_callback_wrapper(LPVOID p)
+{
+    struct queue_timer *t = (struct queue_timer *) p;
+    struct timer_queue *q = t->q;
+
+    t->callback(t->param, TRUE);
+
+    RtlEnterCriticalSection(&q->cs);
+    InterlockedDecrement(&t->runcount);
+    if (t->die && t->runcount == 0)
+    {
+        queue_remove_timer(q, t);
+        /* Wake up the cleanup loop so it can see if all timers are gone.  */
+        if (q->quit)
+            SetEvent(q->event);
+    }
+    RtlLeaveCriticalSection(&q->cs);
+
+    return 0;
+}
+
+static void queue_timer_expire(struct queue_timer *t)
+{
+    /* We MUST hold the queue cs while calling this function.  */
+    if (!t->die)
+    {
+        ULONG flags =
+            t->flags & (WT_EXECUTEINIOTHREAD | WT_EXECUTEINPERSISTENTTHREAD
+                        | WT_EXECUTELONGFUNCTION | WT_TRANSFER_IMPERSONATION);
+        InterlockedIncrement(&t->runcount);
+        QueueUserWorkItem(timer_callback_wrapper, t, flags);
+    }
+
+    if (t->period)
+    {
+        /* FIXME: Solve this, here or in timer_queue_update.  */
+        DWORD tick = GetTickCount();
+        t->expire = tick + t->period;
+        if (t->expire < tick)
+            ERR("next expiration wrapped\n");
+    }
+    else
+        t->die = TRUE;
+}
+
+static void timer_queue_update(struct timer_queue *q)
+{
+    /* We MUST hold the queue cs while calling this function.  */
+    struct queue_timer *t, *temp, *next = NULL;
+    DWORD tick, time = ~(DWORD) 0;
+
+    LIST_FOR_EACH_ENTRY_SAFE(t, temp, &q->timers, struct queue_timer, entry)
+        if (t->die)
+        {
+            if (t->runcount == 0)
+                queue_remove_timer(q, t);
+        }
+        else if (t->expire < time)
+        {
+            time = t->expire;
+            next = t;
+        }
+
+    tick = GetTickCount();
+    q->timeout = next ? (time < tick ? 0 : time - tick) : INFINITE;
+    q->next = next;
+}
+
+static DWORD WINAPI timer_queue_thread_proc(LPVOID p)
+{
+    struct timer_queue *q = p;
+    BOOL done;
+
+    while (!q->quit)
+    {
+        DWORD ret = WaitForSingleObject(q->event, q->timeout);
+        RtlEnterCriticalSection(&q->cs);
+        if (ret == WAIT_TIMEOUT && q->next)
+            queue_timer_expire(q->next);
+        timer_queue_update(q);
+        RtlLeaveCriticalSection(&q->cs);
+    }
+
+    done = FALSE;
+    while (!done)
+    {
+        RtlEnterCriticalSection(&q->cs);
+        timer_queue_update(q);
+        if (list_count(&q->timers) == 0)
+            done = TRUE;
+        RtlLeaveCriticalSection(&q->cs);
+        if (!done)
+            WaitForSingleObject(q->event, INFINITE);
+    }
+
+    CloseHandle(q->event);
+    CloseHandle(q->thread);
+    HeapFree(GetProcessHeap(), 0, q);
+    return 0;
+}
+
 /***********************************************************************
  *           CreateTimerQueue  (KERNEL32.@)
  */
@@ -1065,15 +1194,27 @@ HANDLE WINAPI CreateTimerQueue(void)
         SetLastError(ERROR_NOT_ENOUGH_MEMORY);
         return NULL;
     }
+
     RtlInitializeCriticalSection(&q->cs);
     list_init(&q->timers);
-    return q;
-}
+    q->next = NULL;
+    q->quit = FALSE;
+    q->timeout = INFINITE;
+    q->event = CreateEventW(NULL, FALSE, FALSE, NULL);
+    if (!q->event)
+    {
+        HeapFree(GetProcessHeap(), 0, q);
+        return NULL;
+    }
+    q->thread = CreateThread(NULL, 0, timer_queue_thread_proc, q, 0, NULL);
+    if (!q->thread)
+    {
+        CloseHandle(q->event);
+        HeapFree(GetProcessHeap(), 0, q);
+        return NULL;
+    }
 
-static void queue_remove_timer(struct timer_queue *q, struct queue_timer *t)
-{
-    list_remove(&t->entry);
-    HeapFree(GetProcessHeap(), 0, t);
+    return q;
 }
 
 /***********************************************************************
@@ -1085,22 +1226,32 @@ BOOL WINAPI DeleteTimerQueueEx(HANDLE TimerQueue, HANDLE CompletionEvent)
     struct queue_timer *t, *temp;
     BOOL ret;
 
+    RtlEnterCriticalSection(&q->cs);
+    LIST_FOR_EACH_ENTRY_SAFE(t, temp, &q->timers, struct queue_timer, entry)
+        t->die = TRUE;
+    q->quit = TRUE;
+    RtlLeaveCriticalSection(&q->cs);
+    SetEvent(q->event);
+
     if (CompletionEvent == INVALID_HANDLE_VALUE)
     {
         ret = TRUE;
+        WaitForSingleObject(q->thread, INFINITE);
     }
     else
     {
+        if (CompletionEvent)
+        {
+            /* FIXME: This should add the event to a list of events in the
+               queue that it will signal when done.  */
+            FIXME("returning and signaling events unimplemented; waiting instead\n");
+            WaitForSingleObject(q->thread, INFINITE);
+            SetEvent(CompletionEvent);
+        }
         ret = FALSE;
         SetLastError(ERROR_IO_PENDING);
     }
 
-    RtlEnterCriticalSection(&q->cs);
-    LIST_FOR_EACH_ENTRY_SAFE(t, temp, &q->timers, struct queue_timer, entry)
-        queue_remove_timer(q, t);
-    RtlLeaveCriticalSection(&q->cs);
-
-    HeapFree(GetProcessHeap(), 0, q);
     return ret;
 }
 
@@ -1123,19 +1274,50 @@ BOOL WINAPI CreateTimerQueueTimer( PHANDLE phNewTimer, HANDLE TimerQueue,
 {
     struct timer_queue *q = TimerQueue;
     struct queue_timer *t = HeapAlloc(GetProcessHeap(), 0, sizeof *t);
+    BOOL ret;
+
     if (!t)
     {
         SetLastError(ERROR_NOT_ENOUGH_MEMORY);
         return FALSE;
     }
-    FIXME(": Timer expiration unimplemented\n");
+    t->q = q;
+    t->runcount = 0;
+    t->callback = Callback;
+    t->param = Parameter;
+    t->period = Period;
+    t->flags = Flags;
+    if (Flags & WT_EXECUTEINTIMERTHREAD)
+        FIXME("WT_EXECUTEINTIMERTHREAD unimplemented\n");
+    {
+        /* FIXME: Solve this, here or in timer_queue_update.  */
+        DWORD tick = GetTickCount();
+        t->expire = tick + DueTime;
+        if (t->expire < tick)
+            ERR("due time wrapped\n");
+    }
+    t->die = FALSE;
 
+    ret = TRUE;
     RtlEnterCriticalSection(&q->cs);
-    list_add_tail(&q->timers, &t->entry);
+    if (q->quit)
+        ret = FALSE;
+    else
+        list_add_tail(&q->timers, &t->entry);
     RtlLeaveCriticalSection(&q->cs);
 
-    *phNewTimer = t;
-    return TRUE;
+    if (ret)
+    {
+        SetEvent(q->event);
+        *phNewTimer = t;
+    }
+    else
+    {
+        SetLastError(ERROR_INVALID_HANDLE);
+        HeapFree(GetProcessHeap(), 0, t);
+    }
+
+    return ret;
 }
 
 /***********************************************************************
diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c
index 0ff3862..71c02a0 100644
--- a/dlls/kernel32/tests/sync.c
+++ b/dlls/kernel32/tests/sync.c
@@ -572,9 +572,11 @@ static void CALLBACK timer_queue_cb2(PVOID p, BOOLEAN timedOut)
             /* Note, XP SP2 does *not* do any deadlock checking, so passing
                INVALID_HANDLE_VALUE here will just hang.  */
             ret = pDeleteTimerQueueTimer(d->q, d->t, NULL);
-            ok(!ret, "DeleteTimerQueueTimer\n");
             todo_wine
+            {
+            ok(!ret, "DeleteTimerQueueTimer\n");
             ok(GetLastError() == ERROR_IO_PENDING, "DeleteTimerQueueTimer\n");
+            }
         }
 }
 
@@ -594,11 +596,30 @@ static void CALLBACK timer_queue_cb3(PVOID p, BOOLEAN timedOut)
         }
 }
 
+static void CALLBACK timer_queue_cb4(PVOID p, BOOLEAN timedOut)
+{
+    struct timer_queue_data1 *d = (struct timer_queue_data1 *) p;
+    ok(timedOut, "Timer callbacks should always time out\n");
+    /* Wait until our own timer handle is set to avoid a pretty much
+       impossible race condition (better safe than sorry).  */
+    if (d->t)
+    {
+        /* This tests whether a timer gets flagged for deletion before
+           or after the callback runs.  If we start this timer with a
+           period of zero (run once), then ChangeTimerQueueTimer will
+           fail if the timer is already flagged.  Hence we really run
+           only once.  Otherwise we will run multiple times.  */
+        BOOL ret = pChangeTimerQueueTimer(d->q, d->t, 50, 50);
+        ok(ret, "ChangeTimerQueueTimer\n");
+        ++d->num_calls;
+    }
+}
+
 static void test_timer_queue(void)
 {
     HANDLE q, t1, t2, t3, t4, t5;
     int n1, n2, n3, n4, n5;
-    struct timer_queue_data1 d2, d3;
+    struct timer_queue_data1 d2, d3, d4;
     HANDLE e;
     BOOL ret;
 
@@ -667,13 +688,9 @@ static void test_timer_queue(void)
 
     ret = pDeleteTimerQueueEx(q, INVALID_HANDLE_VALUE);
     ok(ret, "DeleteTimerQueueEx\n");
-    todo_wine
-    {
     ok(n1 == 1, "Timer callback 1\n");
     ok(n2 < n3, "Timer callback 2 should be much slower than 3\n");
-    }
     ok(n4 == 0, "Timer callback 4\n");
-    todo_wine
     ok(n5 == 1, "Timer callback 5\n");
 
     /* Test syncronous deletion of the queue with event trigger.  */
@@ -691,11 +708,8 @@ static void test_timer_queue(void)
     ret = pDeleteTimerQueueEx(q, e);
     ok(!ret, "DeleteTimerQueueEx\n");
     ok(GetLastError() == ERROR_IO_PENDING, "DeleteTimerQueueEx\n");
-    todo_wine
-    {
     ok(WaitForSingleObject(e, 250) == WAIT_OBJECT_0,
        "Timer destruction event not triggered\n");
-    }
     CloseHandle(e);
 
     /* Test deleting/changing a timer in execution.  */
@@ -722,6 +736,15 @@ static void test_timer_queue(void)
     ok(ret, "CreateTimerQueueTimer\n");
     ok(t3 != NULL, "CreateTimerQueueTimer\n");
 
+    d4.t = t4 = NULL;
+    d4.num_calls = 0;
+    d4.q = q;
+    ret = pCreateTimerQueueTimer(&t4, q, timer_queue_cb4, &d4, 10,
+                                 0, 0);
+    d4.t = t4;
+    ok(ret, "CreateTimerQueueTimer\n");
+    ok(t4 != NULL, "CreateTimerQueueTimer\n");
+
     Sleep(200);
 
     ret = pDeleteTimerQueueEx(q, INVALID_HANDLE_VALUE);
@@ -731,6 +754,7 @@ static void test_timer_queue(void)
     ok(d2.num_calls == d2.max_calls, "DeleteTimerQueueTimer\n");
     ok(d3.num_calls == d3.max_calls, "ChangeTimerQueueTimer\n");
     }
+    ok(d4.num_calls == 1, "Timer flagged for deletion incorrectly\n");
 }
 
 START_TEST(sync)



More information about the wine-patches mailing list