[PATCH v3 0/2] MR10715: mf: Call GetCorrelatedTime without critical section.

[Brendan McGrath (＠redmcg)]

This MR addresses two issues:
1. the MR presentation clock's periodic callback continuing to be called after it was shutdown; and
2. a deadlock that could occur when calling GetCorrelatedTime in parallel with a session shutdown

The deadlock occurs when SAR is the time source. A call to GetCorrelatedTime will result
in an attempt to acquire SAR's critical section. This means this thread
will hold the presentation clock critical section and then attempt
to obtain SARs critical section.

However, if SAR is in the process of shutdown in a parallel thread, then
that thread will already hold the SAR critical section and call
IMFPresentationClock::RemoveClockStateSink. This will attempt to acquire
the presentation clock critical section.

In other words, we have two threads trying to acquire the same two
critical sections in the opposite order. The result is both threads
waiting on the other and thus a deadlock.

This commit resolves the deadlock by ensuring the presentation clock
does not hold its own critical section when calling GetCorrelatedTime.

--
  v3: mf: Call GetCorrelatedTime without critical section.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10715

[PATCH v3 1/2] mf: Remove periodic callback on clock shutdown.

[Brendan McGrath]

From: Brendan McGrath <[e-mail]>

---
 dlls/mf/clock.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/dlls/mf/clock.c b/dlls/mf/clock.c
index 219ef4384e2..f1aead0fcb5 100644
--- a/dlls/mf/clock.c
+++ b/dlls/mf/clock.c
@@ -974,6 +974,11 @@ static HRESULT WINAPI present_clock_shutdown_Shutdown(IMFShutdown *iface)
 
     EnterCriticalSection(&clock->cs);
     clock->is_shut_down = TRUE;
+    if (clock->key)
+    {
+        MFRemovePeriodicCallback(clock->key);
+        clock->key = 0;
+    }
     LeaveCriticalSection(&clock->cs);
 
     return S_OK;
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10715

[PATCH v3 2/2] mf: Call GetCorrelatedTime without critical section.

[Brendan McGrath]

From: Brendan McGrath <[e-mail]>

When SAR is the time source, the call to GetCorrelatedTime will result
in an attempt to acquire SAR's critical section. This means this thread
will hold the presentation clock critical section and then attempt
to obtain SARs critical section.

However, if SAR is in the process of shutdown in a parallel thread, then
that thread will already hold the SAR critical section and call
IMFPresentationClock::RemoveClockStateSink. This will attempt to acquire
the presentation clock critical section.

In other words, we have two threads trying to acquire the same two
critical sections in the opposite order. The result is both threads
waiting on the other and thus a deadlock.

This commit resolves the deadlock by ensuring the presentation clock
does not hold its own critical section when calling GetCorrelatedTime.
---
 dlls/mf/clock.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/dlls/mf/clock.c b/dlls/mf/clock.c
index f1aead0fcb5..1619f2790eb 100644
--- a/dlls/mf/clock.c
+++ b/dlls/mf/clock.c
@@ -565,21 +565,31 @@ static struct clock_timer *presentation_clock_next_timer(struct presentation_clo
 static void CALLBACK presentation_clock_timer_callback(IUnknown *context)
 {
     struct presentation_clock *clock = impl_from_IMFPresentationClock((IMFPresentationClock*)context);
+    IMFPresentationTimeSource *time_source;
     struct clock_timer *timer;
     LONGLONG time, systime;
 
     EnterCriticalSection(&clock->cs);
-    if (clock->time_source &&
-            SUCCEEDED(IMFPresentationTimeSource_GetCorrelatedTime(clock->time_source, 0, &time, &systime)))
+    if ((time_source = clock->time_source))
+        IMFPresentationTimeSource_AddRef(time_source);
+    LeaveCriticalSection(&clock->cs);
+
+    /* We don't hold the critical section during a call to GetCorrelatedTime as it can result in a deadlock. */
+    if (time_source &&
+            SUCCEEDED(IMFPresentationTimeSource_GetCorrelatedTime(time_source, 0, &time, &systime)))
     {
+        EnterCriticalSection(&clock->cs);
         while ( (timer = presentation_clock_next_timer(clock, time)) )
         {
             list_remove(&timer->entry);
             MFInvokeCallback(timer->result);
             IUnknown_Release(&timer->IUnknown_iface);
         }
+        LeaveCriticalSection(&clock->cs);
     }
-    LeaveCriticalSection(&clock->cs);
+
+    if (time_source)
+        IMFPresentationTimeSource_Release(time_source);
 }
 
 static HRESULT clock_change_state(struct presentation_clock *clock, enum clock_command command,
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10715

[Brendan McGrath (＠redmcg)]

On Fri May  8 12:22:27 2026 +0000, Nikolay Sivov wrote:
> I think at least this part should still be protected.
> Oh of course. That is done now. Thank you.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10715#note_139390

[Brendan McGrath (＠redmcg)]

v3:
- Hold critical section during assignment to local variable and adding ref

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10715#note_139391

[PATCH v3 0/2] MR10715: mf: Call GetCorrelatedTime without critical section. - approved

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10715