[PATCH 1/4] quartz/systemclock: Avoid using the sink structure pointer as a cookie.
The same pointer address may be allocated to multiple different structures. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/quartz/systemclock.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/quartz/systemclock.c b/dlls/quartz/systemclock.c index 0cc6d87f84..f41cdbe534 100644 --- a/dlls/quartz/systemclock.c +++ b/dlls/quartz/systemclock.c @@ -26,11 +26,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(quartz); +static int cookie_counter; + struct advise_sink { struct list entry; HANDLE handle; REFERENCE_TIME due_time, period; + int cookie; }; typedef struct SystemClockImpl { @@ -228,6 +231,7 @@ static HRESULT WINAPI SystemClockImpl_AdviseTime(IReferenceClock *iface, sink->handle = (HANDLE)event; sink->due_time = base + offset; sink->period = 0; + sink->cookie = InterlockedIncrement(&cookie_counter); EnterCriticalSection(&clock->cs); insert_advise_sink(sink, &clock->single_sinks); @@ -235,7 +239,7 @@ static HRESULT WINAPI SystemClockImpl_AdviseTime(IReferenceClock *iface, notify_thread(clock); - *cookie = (DWORD_PTR)sink; + *cookie = sink->cookie; return S_OK; } @@ -263,6 +267,7 @@ static HRESULT WINAPI SystemClockImpl_AdvisePeriodic(IReferenceClock* iface, sink->handle = (HANDLE)semaphore; sink->due_time = start; sink->period = period; + sink->cookie = InterlockedIncrement(&cookie_counter); EnterCriticalSection(&clock->cs); insert_advise_sink(sink, &clock->periodic_sinks); @@ -270,7 +275,7 @@ static HRESULT WINAPI SystemClockImpl_AdvisePeriodic(IReferenceClock* iface, notify_thread(clock); - *cookie = (DWORD_PTR)sink; + *cookie = sink->cookie; return S_OK; } @@ -285,7 +290,7 @@ static HRESULT WINAPI SystemClockImpl_Unadvise(IReferenceClock *iface, DWORD_PTR LIST_FOR_EACH_ENTRY(sink, &clock->single_sinks, struct advise_sink, entry) { - if (sink == (struct advise_sink *)cookie) + if (sink->cookie == cookie) { list_remove(&sink->entry); heap_free(sink); @@ -296,7 +301,7 @@ static HRESULT WINAPI SystemClockImpl_Unadvise(IReferenceClock *iface, DWORD_PTR LIST_FOR_EACH_ENTRY(sink, &clock->periodic_sinks, struct advise_sink, entry) { - if (sink == (struct advise_sink *)cookie) + if (sink->cookie == cookie) { list_remove(&sink->entry); heap_free(sink); -- 2.20.1
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/quartz/systemclock.c | 116 +++++++++++++------------------------- 1 file changed, 39 insertions(+), 77 deletions(-) diff --git a/dlls/quartz/systemclock.c b/dlls/quartz/systemclock.c index f41cdbe534..f22c781a3b 100644 --- a/dlls/quartz/systemclock.c +++ b/dlls/quartz/systemclock.c @@ -45,8 +45,7 @@ typedef struct SystemClockImpl { REFERENCE_TIME last_time; CRITICAL_SECTION cs; - /* These lists are ordered by expiration time (soonest first). */ - struct list single_sinks, periodic_sinks; + struct list sinks; } SystemClockImpl; static inline SystemClockImpl *impl_from_IReferenceClock(IReferenceClock *iface) @@ -54,73 +53,48 @@ static inline SystemClockImpl *impl_from_IReferenceClock(IReferenceClock *iface) return CONTAINING_RECORD(iface, SystemClockImpl, IReferenceClock_iface); } -static void insert_advise_sink(struct advise_sink *sink, struct list *queue) +static DWORD WINAPI SystemClockAdviseThread(void *param) { - REFERENCE_TIME due_time = sink->due_time + sink->period; - struct advise_sink *cursor; + SystemClockImpl *clock = param; + struct advise_sink *sink, *cursor; + REFERENCE_TIME current_time; + DWORD timeout = INFINITE; + HANDLE handles[2] = {clock->stop_event, clock->notify_event}; - LIST_FOR_EACH_ENTRY(cursor, queue, struct advise_sink, entry) - { - if (cursor->due_time + cursor->period > due_time) - { - list_add_before(&cursor->entry, &sink->entry); - return; - } - } - - list_add_tail(queue, &sink->entry); -} - -static DWORD WINAPI SystemClockAdviseThread(LPVOID lpParam) { - SystemClockImpl* This = lpParam; - struct advise_sink *sink, *cursor; - struct list *entry; - DWORD timeOut = INFINITE; - REFERENCE_TIME curTime; - HANDLE handles[2] = {This->stop_event, This->notify_event}; - - TRACE("(%p): Main Loop\n", This); + TRACE("Starting advise thread for clock %p.\n", clock); - while (TRUE) { - EnterCriticalSection(&This->cs); - - curTime = GetTickCount64() * 10000; - - /** First SingleShots Advice: sorted list */ - LIST_FOR_EACH_ENTRY_SAFE(sink, cursor, &This->single_sinks, struct advise_sink, entry) + for (;;) { - if (sink->due_time + sink->period > curTime) - break; + EnterCriticalSection(&clock->cs); - SetEvent(sink->handle); - list_remove(&sink->entry); - heap_free(sink); - } + current_time = GetTickCount64() * 10000; - if ((entry = list_head(&This->single_sinks))) - { - sink = LIST_ENTRY(entry, struct advise_sink, entry); - timeOut = (sink->due_time + sink->period - curTime) / 10000; - } - else timeOut = INFINITE; - - /** Now Periodics Advice: semi sorted list (sort cannot be used) */ - LIST_FOR_EACH_ENTRY(sink, &This->periodic_sinks, struct advise_sink, entry) - { - if (sink->due_time <= curTime) - { - DWORD periods = ((curTime - sink->due_time) / sink->period) + 1; - ReleaseSemaphore(sink->handle, periods, NULL); - sink->due_time += periods * sink->period; - } - timeOut = min(timeOut, (sink->due_time - curTime) / 10000); - } + LIST_FOR_EACH_ENTRY_SAFE(sink, cursor, &clock->sinks, struct advise_sink, entry) + { + if (sink->due_time <= current_time) + { + if (sink->period) + { + DWORD periods = ((current_time - sink->due_time) / sink->period) + 1; + ReleaseSemaphore(sink->handle, periods, NULL); + sink->due_time += periods * sink->period; + } + else + { + SetEvent(sink->handle); + list_remove(&sink->entry); + heap_free(sink); + } + } + + timeout = min(timeout, (sink->due_time - current_time) / 10000); + } - LeaveCriticalSection(&This->cs); + LeaveCriticalSection(&clock->cs); - if (WaitForMultipleObjects(2, handles, FALSE, timeOut) == 0) - return 0; - } + if (WaitForMultipleObjects(2, handles, FALSE, timeout) == 0) + return 0; + } } static void notify_thread(SystemClockImpl *clock) @@ -234,7 +208,7 @@ static HRESULT WINAPI SystemClockImpl_AdviseTime(IReferenceClock *iface, sink->cookie = InterlockedIncrement(&cookie_counter); EnterCriticalSection(&clock->cs); - insert_advise_sink(sink, &clock->single_sinks); + list_add_tail(&clock->sinks, &sink->entry); LeaveCriticalSection(&clock->cs); notify_thread(clock); @@ -270,7 +244,7 @@ static HRESULT WINAPI SystemClockImpl_AdvisePeriodic(IReferenceClock* iface, sink->cookie = InterlockedIncrement(&cookie_counter); EnterCriticalSection(&clock->cs); - insert_advise_sink(sink, &clock->periodic_sinks); + list_add_tail(&clock->sinks, &sink->entry); LeaveCriticalSection(&clock->cs); notify_thread(clock); @@ -288,18 +262,7 @@ static HRESULT WINAPI SystemClockImpl_Unadvise(IReferenceClock *iface, DWORD_PTR EnterCriticalSection(&clock->cs); - LIST_FOR_EACH_ENTRY(sink, &clock->single_sinks, struct advise_sink, entry) - { - if (sink->cookie == cookie) - { - list_remove(&sink->entry); - heap_free(sink); - LeaveCriticalSection(&clock->cs); - return S_OK; - } - } - - LIST_FOR_EACH_ENTRY(sink, &clock->periodic_sinks, struct advise_sink, entry) + LIST_FOR_EACH_ENTRY(sink, &clock->sinks, struct advise_sink, entry) { if (sink->cookie == cookie) { @@ -339,8 +302,7 @@ HRESULT QUARTZ_CreateSystemClock(IUnknown *outer, void **out) } object->IReferenceClock_iface.lpVtbl = &SystemClock_Vtbl; - list_init(&object->single_sinks); - list_init(&object->periodic_sinks); + list_init(&object->sinks); InitializeCriticalSection(&object->cs); object->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": SystemClockImpl.cs"); -- 2.20.1
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/quartz/systemclock.c | 42 ++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/dlls/quartz/systemclock.c b/dlls/quartz/systemclock.c index f22c781a3b..47f0daddbc 100644 --- a/dlls/quartz/systemclock.c +++ b/dlls/quartz/systemclock.c @@ -108,29 +108,31 @@ static void notify_thread(SystemClockImpl *clock) SetEvent(clock->notify_event); } -static ULONG WINAPI SystemClockImpl_AddRef(IReferenceClock* iface) { - SystemClockImpl *This = impl_from_IReferenceClock(iface); - ULONG ref = InterlockedIncrement(&This->ref); - - TRACE("(%p): AddRef from %d\n", This, ref - 1); +static HRESULT WINAPI SystemClockImpl_QueryInterface(IReferenceClock *iface, REFIID iid, void **out) +{ + SystemClockImpl *clock = impl_from_IReferenceClock(iface); + TRACE("clock %p, iid %s, out %p.\n", clock, debugstr_guid(iid), out); + + if (IsEqualGUID(iid, &IID_IUnknown) || IsEqualGUID(iid, &IID_IReferenceClock)) + { + IReferenceClock_AddRef(iface); + *out = iface; + return S_OK; + } - return ref; + WARN("%s not implemented, returning E_NOINTERFACE.\n", debugstr_guid(iid)); + *out = NULL; + return E_NOINTERFACE; } -static HRESULT WINAPI SystemClockImpl_QueryInterface(IReferenceClock* iface, REFIID riid, void** ppobj) { - SystemClockImpl *This = impl_from_IReferenceClock(iface); - TRACE("(%p, %s,%p)\n", This, debugstr_guid(riid), ppobj); - - if (IsEqualIID (riid, &IID_IUnknown) || - IsEqualIID (riid, &IID_IReferenceClock)) { - SystemClockImpl_AddRef(iface); - *ppobj = &This->IReferenceClock_iface; - return S_OK; - } - - *ppobj = NULL; - WARN("(%p, %s,%p): not found\n", This, debugstr_guid(riid), ppobj); - return E_NOINTERFACE; +static ULONG WINAPI SystemClockImpl_AddRef(IReferenceClock *iface) +{ + SystemClockImpl *clock = impl_from_IReferenceClock(iface); + ULONG refcount = InterlockedIncrement(&clock->ref); + + TRACE("%p increasing refcount to %u.\n", clock, refcount); + + return refcount; } static ULONG WINAPI SystemClockImpl_Release(IReferenceClock *iface) -- 2.20.1
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/quartz/systemclock.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/dlls/quartz/systemclock.c b/dlls/quartz/systemclock.c index 47f0daddbc..239155ea78 100644 --- a/dlls/quartz/systemclock.c +++ b/dlls/quartz/systemclock.c @@ -36,7 +36,8 @@ struct advise_sink int cookie; }; -typedef struct SystemClockImpl { +struct system_clock +{ IReferenceClock IReferenceClock_iface; LONG ref; @@ -46,16 +47,16 @@ typedef struct SystemClockImpl { CRITICAL_SECTION cs; struct list sinks; -} SystemClockImpl; +}; -static inline SystemClockImpl *impl_from_IReferenceClock(IReferenceClock *iface) +static inline struct system_clock *impl_from_IReferenceClock(IReferenceClock *iface) { - return CONTAINING_RECORD(iface, SystemClockImpl, IReferenceClock_iface); + return CONTAINING_RECORD(iface, struct system_clock, IReferenceClock_iface); } static DWORD WINAPI SystemClockAdviseThread(void *param) { - SystemClockImpl *clock = param; + struct system_clock *clock = param; struct advise_sink *sink, *cursor; REFERENCE_TIME current_time; DWORD timeout = INFINITE; @@ -97,7 +98,7 @@ static DWORD WINAPI SystemClockAdviseThread(void *param) } } -static void notify_thread(SystemClockImpl *clock) +static void notify_thread(struct system_clock *clock) { if (!InterlockedCompareExchange(&clock->thread_created, TRUE, FALSE)) { @@ -110,7 +111,7 @@ static void notify_thread(SystemClockImpl *clock) static HRESULT WINAPI SystemClockImpl_QueryInterface(IReferenceClock *iface, REFIID iid, void **out) { - SystemClockImpl *clock = impl_from_IReferenceClock(iface); + struct system_clock *clock = impl_from_IReferenceClock(iface); TRACE("clock %p, iid %s, out %p.\n", clock, debugstr_guid(iid), out); if (IsEqualGUID(iid, &IID_IUnknown) || IsEqualGUID(iid, &IID_IReferenceClock)) @@ -127,7 +128,7 @@ static HRESULT WINAPI SystemClockImpl_QueryInterface(IReferenceClock *iface, REF static ULONG WINAPI SystemClockImpl_AddRef(IReferenceClock *iface) { - SystemClockImpl *clock = impl_from_IReferenceClock(iface); + struct system_clock *clock = impl_from_IReferenceClock(iface); ULONG refcount = InterlockedIncrement(&clock->ref); TRACE("%p increasing refcount to %u.\n", clock, refcount); @@ -137,7 +138,7 @@ static ULONG WINAPI SystemClockImpl_AddRef(IReferenceClock *iface) static ULONG WINAPI SystemClockImpl_Release(IReferenceClock *iface) { - SystemClockImpl *clock = impl_from_IReferenceClock(iface); + struct system_clock *clock = impl_from_IReferenceClock(iface); ULONG refcount = InterlockedDecrement(&clock->ref); TRACE("%p decreasing refcount to %u.\n", clock, refcount); @@ -161,7 +162,7 @@ static ULONG WINAPI SystemClockImpl_Release(IReferenceClock *iface) static HRESULT WINAPI SystemClockImpl_GetTime(IReferenceClock *iface, REFERENCE_TIME *time) { - SystemClockImpl *clock = impl_from_IReferenceClock(iface); + struct system_clock *clock = impl_from_IReferenceClock(iface); REFERENCE_TIME ret; HRESULT hr; @@ -186,7 +187,7 @@ static HRESULT WINAPI SystemClockImpl_GetTime(IReferenceClock *iface, REFERENCE_ static HRESULT WINAPI SystemClockImpl_AdviseTime(IReferenceClock *iface, REFERENCE_TIME base, REFERENCE_TIME offset, HEVENT event, DWORD_PTR *cookie) { - SystemClockImpl *clock = impl_from_IReferenceClock(iface); + struct system_clock *clock = impl_from_IReferenceClock(iface); struct advise_sink *sink; TRACE("clock %p, base %s, offset %s, event %#lx, cookie %p.\n", @@ -222,7 +223,7 @@ static HRESULT WINAPI SystemClockImpl_AdviseTime(IReferenceClock *iface, static HRESULT WINAPI SystemClockImpl_AdvisePeriodic(IReferenceClock* iface, REFERENCE_TIME start, REFERENCE_TIME period, HSEMAPHORE semaphore, DWORD_PTR *cookie) { - SystemClockImpl *clock = impl_from_IReferenceClock(iface); + struct system_clock *clock = impl_from_IReferenceClock(iface); struct advise_sink *sink; TRACE("clock %p, start %s, period %s, semaphore %#lx, cookie %p.\n", @@ -257,7 +258,7 @@ static HRESULT WINAPI SystemClockImpl_AdvisePeriodic(IReferenceClock* iface, static HRESULT WINAPI SystemClockImpl_Unadvise(IReferenceClock *iface, DWORD_PTR cookie) { - SystemClockImpl *clock = impl_from_IReferenceClock(iface); + struct system_clock *clock = impl_from_IReferenceClock(iface); struct advise_sink *sink; TRACE("clock %p, cookie %#lx.\n", clock, cookie); @@ -293,7 +294,7 @@ static const IReferenceClockVtbl SystemClock_Vtbl = HRESULT QUARTZ_CreateSystemClock(IUnknown *outer, void **out) { - SystemClockImpl *object; + struct system_clock *object; TRACE("outer %p, out %p.\n", outer, out); -- 2.20.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=49487 Your paranoid android. === debian9b (64 bit WoW report) === quartz: Unhandled exception: page fault on read access to 0x00000000 in 64-bit code (0x00007f282fc9a00c). Report errors: quartz:avisplit crashed (c0000005)
participants (2)
-
Marvin -
Zebediah Figura