> I tested this locally by setting the timeout for xz to 0 on line 57 of
> src/xz/coder.c and it deadlocks on a truncated xz file. It seems like a
> race condition because it deadlocks ~90% of the time in my liblzma tests.
> When debugging, I can see that the main thread does not return from
> lzma_code when the LZMA_FINISH action is given. When the decoder receives
> LZMA_FINISH, it should probably signal to the worker threads that no more
> input is coming and to exit with error if they are in the middle of a
> block. Even if LZMA_FINISH is not given, we still need to avoid deadlock at
> all costs. I will come up with a patch for this since it sounds like
> a fun problem :)
As promised, I have attached a patch to solve the problem. Instead of doing as I
had originally proposed, I simply added a wake up signal to a sleeping thread if
partial updates are enabled. When the worker wakes up, it checks if no
more input
is available and signals to the main thread if it has output ready
before going back
to sleep. This prevents the deadlock on my liblzma tests and testing xz
with/without timeout.
Jia Tan
From 5f020915b4e9b60759515df61b9812c1d45c8995 Mon Sep 17 00:00:00 2001
From: jiat75 <jiat0...@gmail.com>
Date: Tue, 15 Mar 2022 20:25:50 +0800
Subject: [PATCH] Deadlock prevention with partial updates in worker thread
These changes prevent deadlock in the mt decoder. The issue
with truncated xz files was the main threaded had a race condition with the
last worker thread. The main thread was telling the worker thread to do
partial updates after the worker thread had already begun sleeping until more
input arrived. The main thread then went to sleep until the worker thread
finished its input. This resulted in deadlock since both threads are sleeping
and waiting on the other to wake it
---
src/liblzma/common/stream_decoder_mt.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/liblzma/common/stream_decoder_mt.c b/src/liblzma/common/stream_decoder_mt.c
index 1f1cd771..11d4cce7 100644
--- a/src/liblzma/common/stream_decoder_mt.c
+++ b/src/liblzma/common/stream_decoder_mt.c
@@ -304,6 +304,9 @@ worker_enable_partial_update(void *thr_ptr)
mythread_sync(thr->mutex) {
thr->partial_update = true;
+ // Signal to worker thread to wake it up
+ // in case it has a partial update ready
+ mythread_cond_signal(&thr->cond);
}
}
@@ -375,6 +378,19 @@ next_loop_unlocked:
if (in_filled == thr->in_pos) {
mythread_cond_wait(&thr->cond, &thr->mutex);
+ // If thr->partial_update is true and we have no new update,
+ // tell the main thread the progress made to avoid a
+ // race condition with the main thread setting partial
+ // update and this thread sleeping until more input
+ // arrives. This is only necessary if there is a truncated
+ // file
+ if (thr->partial_update && in_filled == thr->in_pos) {
+ mythread_sync(thr->coder->mutex) {
+ thr->outbuf->pos = thr->out_pos;
+ thr->outbuf->decoder_in_pos = thr->in_pos;
+ mythread_cond_signal(&thr->coder->cond);
+ }
+ }
goto next_loop_unlocked;
}
--
2.25.1
