winmm: Fixes to timer callback function
Maarten Lankhorst
m.b.lankhorst at gmail.com
Tue Jun 5 12:46:06 CDT 2007
TIME_cbcrst is held when there's a chance of a callback being executed,
a global TIME_wTimerID holds which timer id it is, so if an app wants to
kill it synchronously won't accidentally do it while the function is
still being run. In worst case race condition timeSetKillEvent wants to
hold the TIME_cbcrst lock while another function is being executed by
callback, which is harmless.
-------------- next part --------------
>From 48f9a7afd43292d6bf6d5c7a768db6e8cded8868 Mon Sep 17 00:00:00 2001
From: Maarten Lankhorst <m.b.lankhorst at gmail.com>
Date: Tue, 5 Jun 2007 18:39:29 +0200
Subject: [PATCH] winmm: Fixes to timer callback function
---
dlls/winmm/time.c | 194 +++++++++++++++++++----------------------------------
1 files changed, 70 insertions(+), 124 deletions(-)
diff --git a/dlls/winmm/time.c b/dlls/winmm/time.c
index 27a4e46..7ecbf52 100644
--- a/dlls/winmm/time.c
+++ b/dlls/winmm/time.c
@@ -1,9 +1,8 @@
-/* -*- tab-width: 8; c-basic-offset: 4 -*- */
-
/*
* MMSYSTEM time functions
*
* Copyright 1993 Martin Ayotte
+ * Copyright 2007 Maarten Lankhorst
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -47,6 +46,7 @@ static LPWINE_TIMERENTRY TIME_TimersList;
static HANDLE TIME_hWakeEvent;
static CRITICAL_SECTION TIME_cbcrst;
static BOOL TIME_TimeToDie = TRUE;
+static UINT16 TIME_wTimerID;
/*
* Some observations on the behavior of winmm on Windows.
@@ -77,58 +77,18 @@ static BOOL TIME_TimeToDie = TRUE;
#define MMSYSTIME_MININTERVAL (1)
#define MMSYSTIME_MAXINTERVAL (65535)
-
-static void TIME_TriggerCallBack(LPWINE_TIMERENTRY lpTimer)
-{
- TRACE("%04x:CallBack => lpFunc=%p wTimerID=%04X dwUser=%08X dwTriggerTime %d(delta %d)\n",
- GetCurrentThreadId(), lpTimer->lpFunc, lpTimer->wTimerID, lpTimer->dwUser,
- lpTimer->dwTriggerTime, GetTickCount() - lpTimer->dwTriggerTime);
-
- /* - TimeProc callback that is called here is something strange, under Windows 3.1x it is called
- * during interrupt time, is allowed to execute very limited number of API calls (like
- * PostMessage), and must reside in DLL (therefore uses stack of active application). So I
- * guess current implementation via SetTimer has to be improved upon.
- */
- switch (lpTimer->wFlags & 0x30) {
- case TIME_CALLBACK_FUNCTION:
- if (lpTimer->wFlags & WINE_TIMER_IS32)
- (lpTimer->lpFunc)(lpTimer->wTimerID, 0, lpTimer->dwUser, 0, 0);
- else if (pFnCallMMDrvFunc16)
- pFnCallMMDrvFunc16((DWORD)lpTimer->lpFunc, lpTimer->wTimerID, 0,
- lpTimer->dwUser, 0, 0);
- break;
- case TIME_CALLBACK_EVENT_SET:
- SetEvent((HANDLE)lpTimer->lpFunc);
- break;
- case TIME_CALLBACK_EVENT_PULSE:
- PulseEvent((HANDLE)lpTimer->lpFunc);
- break;
- default:
- FIXME("Unknown callback type 0x%04x for mmtime callback (%p), ignored.\n",
- lpTimer->wFlags, lpTimer->lpFunc);
- break;
- }
-}
-
/**************************************************************************
* TIME_MMSysTimeCallback
*/
static DWORD CALLBACK TIME_MMSysTimeCallback(LPWINE_MM_IDATA iData)
{
-static int nSizeLpTimers;
-static LPWINE_TIMERENTRY lpTimers;
-
- LPWINE_TIMERENTRY timer, *ptimer, *next_ptimer;
- int idx;
- DWORD cur_time;
- DWORD delta_time;
- DWORD ret_time = INFINITE;
- DWORD adjust_time;
-
+ LPWINE_TIMERENTRY timer, *ptimer;
+ WINE_TIMERENTRY ftime;
+ DWORD cur_time, delta_time, adjust_time, ret_time = INFINITE;
/* optimize for the most frequent case - no events */
- if (! TIME_TimersList)
- return(ret_time);
+ if (!TIME_TimersList)
+ return ret_time;
/* since timeSetEvent() and timeKillEvent() can be called
* from 16 bit code, there are cases where win16 lock is
@@ -137,75 +97,74 @@ static LPWINE_TIMERENTRY lpTimers;
* 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.
- * the hKillTimeEvent is used to mark the section where we
- * handle the callbacks so we can do synchronous kills.
- * EPP 99/07/13, updated 04/01/10
+ * To cope with that, copy timer data, and unlock, then restart
+ * entire loop to guarantee consistency
*/
- idx = 0;
cur_time = GetTickCount();
EnterCriticalSection(&iData->cs);
for (ptimer = &TIME_TimersList; *ptimer != NULL; ) {
timer = *ptimer;
- next_ptimer = &timer->lpNext;
if (cur_time >= timer->dwTriggerTime)
{
- if (timer->lpFunc) {
- if (idx == nSizeLpTimers) {
- if (lpTimers)
- lpTimers = (LPWINE_TIMERENTRY)
- HeapReAlloc(GetProcessHeap(), 0, lpTimers,
- ++nSizeLpTimers * sizeof(WINE_TIMERENTRY));
- else
- lpTimers = (LPWINE_TIMERENTRY)
- HeapAlloc(GetProcessHeap(), 0,
- ++nSizeLpTimers * sizeof(WINE_TIMERENTRY));
- }
- lpTimers[idx++] = *timer;
-
- }
-
- /* Update the time after we make the copy to preserve
- the original trigger time */
- timer->dwTriggerTime += timer->wDelay;
-
- /* TIME_ONESHOT is defined as 0 */
if (!(timer->wFlags & TIME_PERIODIC))
{
- /* unlink timer from timers list */
- *ptimer = *next_ptimer;
+ ftime = *timer;
HeapFree(GetProcessHeap(), 0, timer);
-
- /* We don't need to trigger oneshots again */
- delta_time = INFINITE;
+ timer = &ftime;
+ *ptimer = (*ptimer)->lpNext;
}
- else
+
+ timer->dwTriggerTime += timer->wDelay;
+ switch (timer->wFlags & 0x30)
{
- /* Compute when this event needs this function
- to be called again */
- if (timer->dwTriggerTime <= cur_time)
- delta_time = 0;
- else
- delta_time = timer->dwTriggerTime - cur_time;
+ case TIME_CALLBACK_EVENT_SET:
+ SetEvent((HANDLE)timer->lpFunc);
+ break;
+ case TIME_CALLBACK_EVENT_PULSE:
+ PulseEvent((HANDLE)timer->lpFunc);
+ break;
+ case TIME_CALLBACK_FUNCTION:
+ {
+ DWORD_PTR dwUser = timer->dwUser;
+ LPTIMECALLBACK cb = timer->lpFunc;
+ DWORD is32 = timer->wFlags & WINE_TIMER_IS32;
+ TIME_wTimerID = timer->wTimerID;
+
+ /* If a program wants to kill function synchronously, make sure that when
+ * iData->cs is held the right value is available for wTimerID and that
+ * TIME_cbcrst is held while the function has a chance of being executed
+ */
+ EnterCriticalSection(&TIME_cbcrst);
+ LeaveCriticalSection(&iData->cs);
+
+ if (is32)
+ (cb)(TIME_wTimerID, 0, dwUser, 0, 0);
+ else if (pFnCallMMDrvFunc16)
+ pFnCallMMDrvFunc16((DWORD_PTR)cb, TIME_wTimerID, 0, dwUser, 0, 0);
+ LeaveCriticalSection(&TIME_cbcrst);
+
+ EnterCriticalSection(&iData->cs);
+ TIME_wTimerID = 0xffff;
+ ptimer = &TIME_TimersList;
+ break;
+ }
+ default:
+ FIXME("Unknown callback type 0x%04x for mmtime callback (%p), ignored.\n", timer->wFlags, timer->lpFunc);
+ break;
}
- }
- else
- delta_time = timer->dwTriggerTime - cur_time;
+ /* We might need to trigger again */
+ continue;
+ }
+ delta_time = timer->dwTriggerTime - cur_time;
/* Determine when we need to return to this function */
ret_time = min(ret_time, delta_time);
- ptimer = next_ptimer;
+ ptimer = &(*ptimer)->lpNext;
}
LeaveCriticalSection(&iData->cs);
- EnterCriticalSection(&TIME_cbcrst);
- while (idx > 0) TIME_TriggerCallBack(&lpTimers[--idx]);
- LeaveCriticalSection(&TIME_cbcrst);
-
/* Finally, adjust the recommended wait time downward
by the amount of time the processing routines
actually took */
@@ -268,6 +227,7 @@ void TIME_MMTimeStart(void)
SetThreadPriority(TIME_hMMTimer, THREAD_PRIORITY_TIME_CRITICAL);
InitializeCriticalSection(&TIME_cbcrst);
TIME_cbcrst.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": WINMM.TIME_cbcrst");
+ TIME_wTimerID = 0xffff;
}
}
@@ -287,9 +247,6 @@ void TIME_MMTimeStop(void)
CloseHandle(TIME_hMMTimer);
CloseHandle(TIME_hWakeEvent);
TIME_hMMTimer = 0;
- TIME_cbcrst.DebugInfo->Spare[0] = 0;
- DeleteCriticalSection(&TIME_cbcrst);
- TIME_TimersList = NULL;
}
}
@@ -377,32 +334,30 @@ MMRESULT WINAPI timeSetEvent(UINT wDelay, UINT wResol, LPTIMECALLBACK lpFunc,
*/
MMRESULT WINAPI timeKillEvent(UINT wID)
{
- LPWINE_TIMERENTRY lpSelf = NULL, *lpTimer;
+ LPWINE_TIMERENTRY *lpTimer;
TRACE("(%u)\n", wID);
EnterCriticalSection(&WINMM_IData.cs);
/* remove WINE_TIMERENTRY from list */
for (lpTimer = &TIME_TimersList; *lpTimer; lpTimer = &(*lpTimer)->lpNext) {
if (wID == (*lpTimer)->wTimerID) {
- lpSelf = *lpTimer;
- /* unlink timer of id 'wID' */
+ LPWINE_TIMERENTRY lpSelf = *lpTimer;
+ BOOL lock = (TIME_wTimerID == lpSelf->wTimerID);
*lpTimer = (*lpTimer)->lpNext;
- break;
- }
+ LeaveCriticalSection(&WINMM_IData.cs);
+
+ if (lock)
+ EnterCriticalSection(&TIME_cbcrst);
+ HeapFree(GetProcessHeap(), 0, lpSelf);
+ if (lock)
+ LeaveCriticalSection(&TIME_cbcrst);
+ return TIMERR_NOERROR;
+ }
}
LeaveCriticalSection(&WINMM_IData.cs);
- if (!lpSelf)
- {
- WARN("wID=%u is not a valid timer ID\n", wID);
- return MMSYSERR_INVALPARAM;
- }
- if (lpSelf->wFlags & TIME_KILL_SYNCHRONOUS)
- EnterCriticalSection(&TIME_cbcrst);
- HeapFree(GetProcessHeap(), 0, lpSelf);
- if (lpSelf->wFlags & TIME_KILL_SYNCHRONOUS)
- LeaveCriticalSection(&TIME_cbcrst);
- return TIMERR_NOERROR;
+ WARN("wID=%u is not a valid timer ID\n", wID);
+ return MMSYSERR_INVALPARAM;
}
/**************************************************************************
@@ -464,15 +419,6 @@ MMRESULT WINAPI timeEndPeriod(UINT wPeriod)
*/
DWORD WINAPI timeGetTime(void)
{
-#if defined(COMMENTOUTPRIORTODELETING)
- DWORD count;
-
- /* FIXME: releasing the win16 lock here is a temporary hack (I hope)
- * that lets mciavi32.dll run correctly
- */
- if (pFnReleaseThunkLock) pFnReleaseThunkLock(&count);
- if (pFnRestoreThunkLock) pFnRestoreThunkLock(count);
-#endif
-
+ TRACE("\n");
return GetTickCount();
}
--
1.4.4.2
More information about the wine-patches
mailing list