On 2022-03-11 21:59:19 [+0800], Jia Tan wrote:
> > 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.
>
> This fixes it just for xz, but if no timeout is specified it will
> still deadlock.
> I haven't looked at the code enough to understand how to fix it for both,
> but I will start to look into that.
No, it is for everyone. In the timeout case we need to check if the
first thread in line can make progress. If we don't provide data new
data to the thread and the thread consumed everything it had then the
thread won't make progress. If the function gets invoked with 0 new data
then we should return LZMA_OK at which point the upper layer (between XZ
binary and the library) will notice that no progress is made and return
an error to xz.
The above patch is not correct because if you do it as I suggeted then
it is possible that an error is returned because the thread is slow and
did not yet make progress.
> I was following the conversation about the soft and hard memory limiting.
> If a user wanted decoding to fail if it can't be done multithreaded and update
> the memory limit as needed, that can't be done right now. It's a minor issue
> that only matters for liblzma, but it would be nice to be able to update both
> limits after decoding has started. I don't consider this a bug, more like
> missing a nice to have feature. One easy solution is to add a new API
> function to liblzma to update the soft memory limit for the multithreaded
> decoder and do nothing / return an error on all other coders. I will add
> a patch for this if you guys think it is a good idea.
Maybe I missundertand you. But if you set memlimit_stop to the same
value as memlimit_threading then you have either multi threaded
decompression or none at all right?
> Jia Tan
Sebastian
