winmm: Rewrite time.c using windows timer queues
Maarten Lankhorst
m.b.lankhorst at gmail.com
Sun Apr 18 06:30:31 CDT 2010
---
At least 2 installers dynamically have regressions as result of
'winmm: Defer loading drivers until they are requested.', this is
because loading the drivers would increase reference count on winmm,
since they're linked to it. Now that drivers aren't loaded by default
winmm actually gets unloaded and deadlocks because the thread gets
killed. Since the solution to kill the thread after last timer died
was uglier than the problem and wouldn't completely solve the issue I
just gave up and replaced the code to TimerQueue calls since winmm
just duplicated that code. Because the thread is owned by kernel32, it
doesn't have to be deleted in DllMain, and no deadlock can occur.
-------------- next part --------------
From e57c87676ee800fd7e4abde8a34da53cb0a48aec Mon Sep 17 00:00:00 2001
From: Maarten Lankhorst <m.b.lankhorst at gmail.com>
Date: Tue, 6 Apr 2010 23:50:43 +0200
Subject: [PATCH 01/15] winmm: Rewrite time.c using windows timer queues
---
dlls/winmm/time.c | 342 +++++++++++------------------------------------------
1 files changed, 72 insertions(+), 270 deletions(-)
diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c
index ee835c7..3150c18 100644
--- a/dlls/winmm/time.c
+++ b/dlls/winmm/time.c
@@ -20,24 +20,7 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
*/
-#include "config.h"
-#include "wine/port.h"
-
#include <stdarg.h>
-#include <errno.h>
-#include <time.h>
-#ifdef HAVE_SYS_TIME_H
-# include <sys/time.h>
-#endif
-#ifdef HAVE_UNISTD_H
-# include <unistd.h>
-#endif
-#ifdef HAVE_POLL_H
-#include <poll.h>
-#endif
-#ifdef HAVE_SYS_POLL_H
-#include <sys/poll.h>
-#endif
#include "windef.h"
#include "winbase.h"
@@ -50,16 +33,14 @@
WINE_DEFAULT_DEBUG_CHANNEL(mmtime);
-typedef struct tagWINE_TIMERENTRY {
- struct list entry;
- UINT wDelay;
- UINT wResol;
- LPTIMECALLBACK lpFunc; /* can be lots of things */
- DWORD_PTR dwUser;
- UINT16 wFlags;
- UINT16 wTimerID;
- DWORD dwTriggerTime;
-} WINE_TIMERENTRY, *LPWINE_TIMERENTRY;
+typedef struct {
+ struct list entry; /* must be first item */
+ LPTIMECALLBACK lpFunc;
+ DWORD_PTR dwUser;
+ UINT wFlags;
+ UINT wTimerID;
+ HANDLE handle;
+} timer_info;
static struct list timer_list = LIST_INIT(timer_list);
@@ -72,216 +53,49 @@ static CRITICAL_SECTION_DEBUG critsect_debug =
};
static CRITICAL_SECTION TIME_cbcrst = { &critsect_debug, -1, 0, 0, 0, 0 };
-static HANDLE TIME_hMMTimer;
-static BOOL TIME_TimeToDie = TRUE;
-static int TIME_fdWake[2] = { -1, -1 };
-
-/* link timer at the appropriate spot in the list */
-static inline void link_timer( WINE_TIMERENTRY *timer )
-{
- WINE_TIMERENTRY *next;
-
- LIST_FOR_EACH_ENTRY( next, &timer_list, WINE_TIMERENTRY, entry )
- if ((int)(next->dwTriggerTime - timer->dwTriggerTime) >= 0) break;
-
- list_add_before( &next->entry, &timer->entry );
-}
-
-/*
- * Some observations on the behavior of winmm on Windows.
- * First, the call to timeBeginPeriod(xx) can never be used
- * to raise the timer resolution, only lower it.
- *
- * Second, a brief survey of a variety of Win 2k and Win X
- * machines showed that a 'standard' (aka default) timer
- * resolution was 1 ms (Win9x is documented as being 1). However, one
- * machine had a standard timer resolution of 10 ms.
- *
- * Further, if we set our default resolution to 1,
- * the implementation of timeGetTime becomes GetTickCount(),
- * and we can optimize the code to reduce overhead.
- *
- * Additionally, a survey of Event behaviors shows that
- * if we request a Periodic event every 50 ms, then Windows
- * makes sure to trigger that event 20 times in the next
- * second. If delays prevent that from happening on exact
- * schedule, Windows will trigger the events as close
- * to the original schedule as is possible, and will eventually
- * bring the event triggers back onto a schedule that is
- * consistent with what would have happened if there were
- * no delays.
- *
- * Jeremy White, October 2004
- */
#define MMSYSTIME_MININTERVAL (1)
#define MMSYSTIME_MAXINTERVAL (65535)
-#ifdef HAVE_POLL
-
-/**************************************************************************
- * TIME_MMSysTimeCallback
- */
-static int TIME_MMSysTimeCallback(void)
+static void CALLBACK timeCallback(void *arg, BOOLEAN fired)
{
- WINE_TIMERENTRY *timer, *to_free;
- int delta_time;
-
- /* since timeSetEvent() and timeKillEvent() can be called
- * from 16 bit code, there are cases where win16 lock is
- * locked upon entering timeSetEvent(), and then the mm timer
- * critical section is locked. This function cannot call the
- * timer callback with the crit sect locked (because callback
- * may need to acquire Win16 lock, thus providing a deadlock
- * situation).
- * To cope with that, we just copy the WINE_TIMERENTRY struct
- * that need to trigger the callback, and call it without the
- * mm timer crit sect locked.
- */
-
- EnterCriticalSection(&WINMM_cs);
- for (;;)
- {
- struct list *ptr = list_head( &timer_list );
- if (!ptr)
- {
- delta_time = -1;
- break;
- }
-
- timer = LIST_ENTRY( ptr, WINE_TIMERENTRY, entry );
- delta_time = timer->dwTriggerTime - GetTickCount();
- if (delta_time > 0) break;
-
- list_remove( &timer->entry );
- if (timer->wFlags & TIME_PERIODIC)
- {
- timer->dwTriggerTime += timer->wDelay;
- link_timer( timer ); /* restart it */
- to_free = NULL;
- }
- else to_free = timer;
-
- switch(timer->wFlags & (TIME_CALLBACK_EVENT_SET|TIME_CALLBACK_EVENT_PULSE))
- {
- case TIME_CALLBACK_EVENT_SET:
- SetEvent(timer->lpFunc);
- break;
- case TIME_CALLBACK_EVENT_PULSE:
- PulseEvent(timer->lpFunc);
- break;
- case TIME_CALLBACK_FUNCTION:
- {
- DWORD_PTR user = timer->dwUser;
- UINT16 id = timer->wTimerID;
- UINT16 flags = timer->wFlags;
- LPTIMECALLBACK func = timer->lpFunc;
-
- if (flags & TIME_KILL_SYNCHRONOUS) EnterCriticalSection(&TIME_cbcrst);
- LeaveCriticalSection(&WINMM_cs);
-
- func(id, 0, user, 0, 0);
-
- EnterCriticalSection(&WINMM_cs);
- if (flags & TIME_KILL_SYNCHRONOUS) LeaveCriticalSection(&TIME_cbcrst);
+ timer_info *timer = arg, *cur;
+
+ EnterCriticalSection(&TIME_cbcrst);
+ LIST_FOR_EACH_ENTRY( cur, &timer_list, timer_info, entry )
+ if (cur == arg) {
+ switch (timer->wFlags &
+ (TIME_CALLBACK_EVENT_SET|TIME_CALLBACK_EVENT_PULSE)) {
+ case TIME_CALLBACK_EVENT_SET:
+ SetEvent(timer->lpFunc);
+ break;
+ case TIME_CALLBACK_EVENT_PULSE:
+ PulseEvent(timer->lpFunc);
+ break;
+ case TIME_CALLBACK_FUNCTION: {
+ timer->lpFunc(timer->wTimerID, 0, timer->dwUser, 0, 0);
+ break;
+ }
}
break;
}
- HeapFree( GetProcessHeap(), 0, to_free );
- }
- LeaveCriticalSection(&WINMM_cs);
- return delta_time;
-}
-
-/**************************************************************************
- * TIME_MMSysTimeThread
- */
-static DWORD CALLBACK TIME_MMSysTimeThread(LPVOID arg)
-{
- int sleep_time, ret;
- char readme[16];
- struct pollfd pfd;
-
- pfd.fd = TIME_fdWake[0];
- pfd.events = POLLIN;
-
- TRACE("Starting main winmm thread\n");
-
- /* FIXME: As an optimization, we could have
- this thread die when there are no more requests
- pending, and then get recreated on the first
- new event; it's not clear if that would be worth
- it or not. */
-
- while (! TIME_TimeToDie)
- {
- sleep_time = TIME_MMSysTimeCallback();
-
- if (sleep_time == 0)
- continue;
-
- if ((ret = poll(&pfd, 1, sleep_time)) < 0)
- {
- if (errno != EINTR && errno != EAGAIN)
- {
- ERR("Unexpected error in poll: %s(%d)\n", strerror(errno), errno);
- break;
- }
- }
-
- while (ret > 0) ret = read(TIME_fdWake[0], readme, sizeof(readme));
- }
- TRACE("Exiting main winmm thread\n");
- return 0;
-}
-
-/**************************************************************************
- * TIME_MMTimeStart
- */
-static void TIME_MMTimeStart(void)
-{
- if (!TIME_hMMTimer) {
- if (pipe(TIME_fdWake) < 0)
- {
- TIME_fdWake[0] = TIME_fdWake[1] = -1;
- ERR("Cannot create pipe: %s\n", strerror(errno));
- } else {
- fcntl(TIME_fdWake[0], F_SETFL, O_NONBLOCK);
- fcntl(TIME_fdWake[1], F_SETFL, O_NONBLOCK);
- }
- TIME_TimeToDie = FALSE;
- TIME_hMMTimer = CreateThread(NULL, 0, TIME_MMSysTimeThread, NULL, 0, NULL);
- SetThreadPriority(TIME_hMMTimer, THREAD_PRIORITY_TIME_CRITICAL);
- }
+ LeaveCriticalSection(&TIME_cbcrst);
}
-#else /* HAVE_POLL */
-
-static void TIME_MMTimeStart(void)
+void TIME_MMTimeStop(void)
{
- FIXME( "not starting system thread\n" );
-}
+ timer_info *cur;
-#endif /* HAVE_POLL */
+ EnterCriticalSection(&TIME_cbcrst);
+ while ((cur = (timer_info *)list_head(&timer_list))) {
+ ERR("List not empty, still has %u, expect deadlock\n", cur->wTimerID);
-/**************************************************************************
- * TIME_MMTimeStop
- */
-void TIME_MMTimeStop(void)
-{
- if (TIME_hMMTimer) {
- const char a='a';
-
- TIME_TimeToDie = TRUE;
- write(TIME_fdWake[1], &a, sizeof(a));
-
- WaitForSingleObject(TIME_hMMTimer, INFINITE);
- close(TIME_fdWake[0]);
- close(TIME_fdWake[1]);
- TIME_fdWake[0] = TIME_fdWake[1] = -1;
- CloseHandle(TIME_hMMTimer);
- TIME_hMMTimer = 0;
- DeleteCriticalSection(&TIME_cbcrst);
+ LeaveCriticalSection(&TIME_cbcrst);
+ DeleteTimerQueueTimer(NULL, cur->handle, INVALID_HANDLE_VALUE);
+ EnterCriticalSection(&TIME_cbcrst);
+ list_remove(&cur->entry);
+ HeapFree(GetProcessHeap(), 0, cur);
}
+ LeaveCriticalSection(&TIME_cbcrst);
}
/**************************************************************************
@@ -293,56 +107,48 @@ MMRESULT WINAPI timeGetSystemTime(LPMMTIME lpTime, UINT wSize)
if (wSize >= sizeof(*lpTime)) {
lpTime->wType = TIME_MS;
lpTime->u.ms = GetTickCount();
-
}
return 0;
}
-/**************************************************************************
- * timeSetEvent [WINMM.@]
- */
MMRESULT WINAPI timeSetEvent(UINT wDelay, UINT wResol, LPTIMECALLBACK lpFunc,
DWORD_PTR dwUser, UINT wFlags)
{
- WORD wNewID = 0;
- LPWINE_TIMERENTRY lpNewTimer;
- LPWINE_TIMERENTRY lpTimer;
- const char c = 'c';
+ UINT wNewID = 0;
+ timer_info *lpNewTimer;
+ timer_info *lpTimer;
+ DWORD flags, period = 0;
TRACE("(%u, %u, %p, %08lX, %04X);\n", wDelay, wResol, lpFunc, dwUser, wFlags);
if (wDelay < MMSYSTIME_MININTERVAL || wDelay > MMSYSTIME_MAXINTERVAL)
return 0;
- lpNewTimer = HeapAlloc(GetProcessHeap(), 0, sizeof(WINE_TIMERENTRY));
+ lpNewTimer = HeapAlloc(GetProcessHeap(), 0, sizeof(*lpNewTimer));
if (lpNewTimer == NULL)
return 0;
- lpNewTimer->wDelay = wDelay;
- lpNewTimer->dwTriggerTime = GetTickCount() + wDelay;
-
- /* FIXME - wResol is not respected, although it is not clear
- that we could change our precision meaningfully */
- lpNewTimer->wResol = wResol;
lpNewTimer->lpFunc = lpFunc;
lpNewTimer->dwUser = dwUser;
lpNewTimer->wFlags = wFlags;
- EnterCriticalSection(&WINMM_cs);
+ EnterCriticalSection(&TIME_cbcrst);
- LIST_FOR_EACH_ENTRY( lpTimer, &timer_list, WINE_TIMERENTRY, entry )
+ LIST_FOR_EACH_ENTRY( lpTimer, &timer_list, timer_info, entry )
wNewID = max(wNewID, lpTimer->wTimerID);
-
- link_timer( lpNewTimer );
lpNewTimer->wTimerID = wNewID + 1;
- TIME_MMTimeStart();
+ list_add_tail(&timer_list, &lpNewTimer->entry);
- LeaveCriticalSection(&WINMM_cs);
+ flags = WT_EXECUTEINTIMERTHREAD;
+ if (wFlags & TIME_PERIODIC)
+ period = wDelay;
+ else
+ flags |= WT_EXECUTEONLYONCE;
+ CreateTimerQueueTimer(&lpNewTimer->handle, NULL, timeCallback, lpNewTimer, wDelay, period, flags);
- /* Wake the service thread in case there is work to be done */
- write(TIME_fdWake[1], &c, sizeof(c));
+ LeaveCriticalSection(&TIME_cbcrst);
TRACE("=> %u\n", wNewID + 1);
@@ -354,34 +160,30 @@ MMRESULT WINAPI timeSetEvent(UINT wDelay, UINT wResol, LPTIMECALLBACK lpFunc,
*/
MMRESULT WINAPI timeKillEvent(UINT wID)
{
- WINE_TIMERENTRY *lpSelf = NULL, *lpTimer;
- DWORD wFlags;
+ timer_info *cur;
TRACE("(%u)\n", wID);
- EnterCriticalSection(&WINMM_cs);
- /* remove WINE_TIMERENTRY from list */
- LIST_FOR_EACH_ENTRY( lpTimer, &timer_list, WINE_TIMERENTRY, entry )
+ EnterCriticalSection(&TIME_cbcrst);
+ LIST_FOR_EACH_ENTRY(cur, &timer_list, timer_info, entry)
{
- if (wID == lpTimer->wTimerID) {
- lpSelf = lpTimer;
- list_remove( &lpTimer->entry );
- break;
- }
+ if (wID == cur->wTimerID) {
+ HANDLE timer = cur->handle, event;
+
+ if (cur->wFlags & TIME_KILL_SYNCHRONOUS)
+ event = INVALID_HANDLE_VALUE;
+ else
+ event = NULL;
+ list_remove(&cur->entry);
+ HeapFree(GetProcessHeap(), 0, cur);
+ LeaveCriticalSection(&TIME_cbcrst);
+ DeleteTimerQueueTimer(NULL, timer, event);
+ return TIMERR_NOERROR;
+ }
}
- LeaveCriticalSection(&WINMM_cs);
+ LeaveCriticalSection(&TIME_cbcrst);
- if (!lpSelf)
- {
- WARN("wID=%u is not a valid timer ID\n", wID);
- return MMSYSERR_INVALPARAM;
- }
- wFlags = lpSelf->wFlags;
- if (wFlags & TIME_KILL_SYNCHRONOUS)
- EnterCriticalSection(&TIME_cbcrst);
- HeapFree(GetProcessHeap(), 0, lpSelf);
- if (wFlags & TIME_KILL_SYNCHRONOUS)
- LeaveCriticalSection(&TIME_cbcrst);
- return TIMERR_NOERROR;
+ WARN("wID=%u is not a valid timer ID\n", wID);
+ return MMSYSERR_INVALPARAM;
}
/**************************************************************************
--
1.7.0
More information about the wine-patches
mailing list