[FFmpeg-cvslog] avcodec/vvcdec: fix potential deadlock in report_frame_progress

Nuo Mi git at videolan.org
Tue Sep 3 17:12:48 EEST 2024


ffmpeg | branch: master | Nuo Mi <nuomi2021 at gmail.com> | Sun Aug 25 12:35:46 2024 +0800| [3d2fafa2294bf0c1570b278c9d7dd8648a155e06] | committer: Nuo Mi

avcodec/vvcdec: fix potential deadlock in report_frame_progress

Fixes:
https://fate.ffmpeg.org/report.cgi?slot=x86_64-archlinux-gcc-tsan&time=20240823175808

Reproduction steps:
./configure --enable-memory-poisoning --toolchain=gcc-tsan --disable-stripping && make fate-vvc

Root cause:
We hold the current frame's lock while updating progress for other frames,
which also requires acquiring other frame locks. This could potentially lead to a deadlock.
However, I don't think this will happen in practice because progress updates are one-way, with no cyclic dependencies.
But we need this patch to make FATE happy.

> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=3d2fafa2294bf0c1570b278c9d7dd8648a155e06
---

 libavcodec/vvc/refs.c   | 13 +++++++------
 libavcodec/vvc/thread.c |  8 ++++++--
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index c1fc6132c2..bebcef7fd6 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -588,12 +588,13 @@ void ff_vvc_report_progress(VVCFrame *frame, const VVCProgress vp, const int y)
     VVCProgressListener *l = NULL;
 
     ff_mutex_lock(&p->lock);
-
-    av_assert0(p->progress[vp] < y || p->progress[vp] == INT_MAX);
-    p->progress[vp] = y;
-    l = get_done_listener(p, vp);
-    ff_cond_signal(&p->cond);
-
+    if (p->progress[vp] < y) {
+        // Due to the nature of thread scheduling, later progress may reach this point before earlier progress.
+        // Therefore, we only update the progress when p->progress[vp] < y.
+        p->progress[vp] = y;
+        l = get_done_listener(p, vp);
+        ff_cond_signal(&p->cond);
+    }
     ff_mutex_unlock(&p->lock);
 
     while (l) {
diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
index 74f8e4e9d0..86a7753c6a 100644
--- a/libavcodec/vvc/thread.c
+++ b/libavcodec/vvc/thread.c
@@ -466,12 +466,16 @@ static void report_frame_progress(VVCFrameContext *fc,
         y = old = ft->row_progress[idx];
         while (y < ft->ctu_height && atomic_load(&ft->rows[y].col_progress[idx]) == ft->ctu_width)
             y++;
+        if (old != y)
+            ft->row_progress[idx] = y;
+        // ff_vvc_report_progress will acquire other frames' locks, which could lead to a deadlock
+        // We need to unlock ft->lock first
+        ff_mutex_unlock(&ft->lock);
+
         if (old != y) {
             const int progress = y == ft->ctu_height ? INT_MAX : y * ctu_size;
-            ft->row_progress[idx] = y;
             ff_vvc_report_progress(fc->ref, idx, progress);
         }
-        ff_mutex_unlock(&ft->lock);
     }
 }
 



More information about the ffmpeg-cvslog mailing list