[contrib/pzstd] Prevent hangs when there are errors

[yotann]

When two threads are using a WorkQueue and the reader thread exits due to an error, it must call WorkQueue::finish() to wake up the writer thread. Otherwise, if the queue is full and the writer thread is waiting for a free slot, it could hang forever.

This can happen in pratice when decompressing a large, corrupted file that does not contain pzstd skippable frames.

[embg]

Why is in->finish() here not sufficient?

zstd/contrib/pzstd/Pzstd.cpp

Line 405 in 5dc9c3c

auto inGuard = makeScopeGuard([&] { in->finish(); });

I do agree that the changes in writeFile() are necessary.

[embg]

OK, I see what's going on now. In both asyncCompressChunks() and asyncDecompressFrames(), the scope guard is in the same scope as readData(). So the scope guard destructor won't run if readData() is stuck, which can cause a deadlock.

The only remaining question I have is whether it's safe to finish() a WorkQueue twice. I see you added a code comment addressing this question. Will take a look on Monday.

[embg]

OK, I don't see any reason why we can't call finish() twice on the same WorkQueue. Locking twice is safe, setting done_ twice is safe, and notify_all() doesn't have any pre-conditions according to C++ docs.

LGTM.

[terrelln]

Thanks for the PR @yotann & thanks for reviewing @embg!