diff options
author | Nuo Mi <nuomi2021@gmail.com> | 2024-08-25 12:35:46 +0800 |
---|---|---|
committer | Nuo Mi <nuomi2021@gmail.com> | 2024-09-03 21:32:27 +0800 |
commit | 3d2fafa2294bf0c1570b278c9d7dd8648a155e06 (patch) | |
tree | 3fd359bd836bad78331beb4e94e5157549a1e1b2 | |
parent | 54291f43837b23d9baa530652d2f1aa8a22db28e (diff) | |
download | ffmpeg-3d2fafa2294bf0c1570b278c9d7dd8648a155e06.tar.gz |
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.
-rw-r--r-- | libavcodec/vvc/refs.c | 13 | ||||
-rw-r--r-- | 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); } } |