[FFmpeg-devel] [PATCH] avfilter/vsrc_gfxcapture: fix possible missed wakeup race in capture loop (PR #20531)

[FFmpeg-devel] [PATCH] avfilter/vsrc_gfxcapture: fix possible missed wakeup race in capture loop (PR #20531)

[Timo Rothenpieler via ffmpeg-devel]

PR #20531 opened by Timo Rothenpieler (BtbN)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20531
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20531.patch

There is the tiniest chance that the cond var wait function checks the predicate, but before it can enter the wait, the capture thread both updated the checked atomics and signals the cond var.
In that case, the condvar would never be signaled, and an entire frame or even the closing of the captured window might be missed, and the cond var will wait until its timeout.

The frame_arrived_mutex cannot be held over the process_frame_if_exists(), since it also synchronizes with the thread, so this would be a sure deadlock risk.

This change moves the lock only over the condvar wait itself, and makes the callback handlers hold the lock while updating the atomic variables, eliminating the risk for a missed wakeup.

I had previously thought the memory ordering of the atomics was enough to prevent this race, but there is still the off chance that it might race, however unlikely it may be.


>From e7e35207c68be1d4da584445f2e31d97b56c908f Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Tue, 16 Sep 2025 00:13:46 +0200
Subject: [PATCH] avfilter/vsrc_gfxcapture: fix possible missed wakeup race in
 capture loop

---
 libavfilter/vsrc_gfxcapture_winrt.cpp | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vsrc_gfxcapture_winrt.cpp b/libavfilter/vsrc_gfxcapture_winrt.cpp
index 6477e56918..2480aabe56 100644
--- a/libavfilter/vsrc_gfxcapture_winrt.cpp
+++ b/libavfilter/vsrc_gfxcapture_winrt.cpp
@@ -196,12 +196,18 @@ static HRESULT get_activation_factory(GfxCaptureContextCpp *ctx, PCWSTR clsid, T
  ****************************************************/
 
 static void wgc_frame_arrived_handler(const std::unique_ptr<GfxCaptureContextWgc> &wgctx) {
-    wgctx->frame_seq.fetch_add(1, std::memory_order_release);
+    {
+        std::lock_guard lock(wgctx->frame_arrived_mutex);
+        wgctx->frame_seq.fetch_add(1, std::memory_order_release);
+    }
     wgctx->frame_arrived_cond.notify_one();
 }
 
 static void wgc_closed_handler(const std::unique_ptr<GfxCaptureContextWgc> &wgctx) {
-    wgctx->window_closed.store(true, std::memory_order_release);
+    {
+        std::lock_guard lock(wgctx->frame_arrived_mutex);
+        wgctx->window_closed.store(true, std::memory_order_release);
+    }
     wgctx->frame_arrived_cond.notify_one();
 }
 
@@ -1455,8 +1461,6 @@ static int gfxcapture_activate(AVFilterContext *avctx)
     if (!ff_outlink_frame_wanted(outlink))
         return FFERROR_NOT_READY;
 
-    std::unique_lock frame_lock(wgctx->frame_arrived_mutex);
-
     for (;;) {
         uint64_t last_seq = wgctx->frame_seq.load(std::memory_order_acquire);
 
@@ -1469,6 +1473,7 @@ static int gfxcapture_activate(AVFilterContext *avctx)
             break;
         }
 
+        std::unique_lock frame_lock(wgctx->frame_arrived_mutex);
         if (!wgctx->frame_arrived_cond.wait_for(frame_lock, std::chrono::seconds(1), [&]() {
             return wgctx->frame_seq.load(std::memory_order_acquire) != last_seq ||
                    wgctx->window_closed.load(std::memory_order_acquire);
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org