On 2022-03-10 22:16:27 [+0800], Jia Tan wrote:
> I started writing tests in the new framework and I found one bug and
> one issue. If you want to check out the tests I have so far, here is a
> link to check out my progress:
> https://github.com/JiaT75/XZ_Utils_Unofficial/tree/test_multithreaded_decoder
>
> The bug is with truncated xz files. In multithreaded mode, if a file
> has been corrupted and is missing the end, deadlock occurs. An easy
> way to recreate this is by using the truncate command:
> truncate -s 30000 some_multiblock_file.xz
>
> And then:
> xz -dk --verbose some_multiblock_file.xz --threads=2
>
> This will result in a deadlock in multithreaded decoding, but not an
> error in single threaded decoding.
This:
diff --git a/src/liblzma/common/stream_decoder_mt.c
b/src/liblzma/common/stream_decoder_mt.c
--- a/src/liblzma/common/stream_decoder_mt.c
+++ b/src/liblzma/common/stream_decoder_mt.c
@@ -786,7 +786,7 @@ read_output_and_wait(struct lzma_stream_coder *coder,
if (mythread_cond_timedwait(&coder->cond,
&coder->mutex,
wait_abs) != 0) {
- ret = LZMA_TIMED_OUT;
+ ret = LZMA_OK;
break;
}
} else {
Should "fixes" it. At some point the main thread needs to check if the
next thread is able to make progress or not and then return LZMA_OK so
that the upper layer can figure out that no progress is made. Otherwise
it stucks in the LZMA_TIMED_OUT loop.
> The issue is with updating the memlimit with lzma_memlimit_set. As you
> note in your comment in stream_decoder_mt_memconfig there is no way to
> update memlimit_threading. If the memlimit_stop is set very low to
> start, it will force memlimit_threading to be that same small value. I
> could see users wanting to keep memlimit_threading and memlimit_stop
> in sync or have memlimit_threading always be some function of
> memlimit_stop (maybe memlimit_stop / 2 or something). I am not sure
> what the best fix is for this at the moment, but I don't think having
> only one of these values be configurable at runtime is the best idea.
> Especially when the initialization forces memlimit_threading to be at
> most memlimit_stop (which makes sense for almost every situation).
The idea to have on limit to keep things going (no matter what) and the
other to have reasonable limit at which point you don't want threads to
be used.
> I will continue to write more tests and then review the code itself.
> Nice job to both of you for getting this feature as far as it is
> already.
>
> Jia Tan
Sebastian
