#130 p7zip deadlock if files are deleted while archiving

[Anonymous]

Version: p7zip-9.20, p7zip-9.0.4
OS: Mac OS, Linux (did not test Windows)
Core file / backtrace OS: 64-bit Arch Linux

If you run '7za a <archive>.zip <dirname> ' and while the archiving is in progress you delete files in <dirname> which were about to be added
into the archive, 7za deadlocks trying to output the WARNING message about the files not being found.

Running 7za in single threaded mode ( -mmt=off ) works fine.

For simplicity, I'm reproducing it with -mmt=2.

Attaching:
1) Python script to reproduce. Run it inside the p7zip source tree after building 7za. Not guaranteed to always work as this is a race condition but
works for me most of the time.
2) Core file
3) Patch that I think fixes the bug. I'm sure you'll find a more elegant way to code it, though.

Looks like the main thread is trying to acquire a lock it already has.

(gdb) up 4
#4 NArchive::NZip::CMtProgressMixer2::SetProgressOffset (this=0x117af60, progressOffset=853) at ../../Archive/Zip/ZipUpdate.cpp:282
282 CriticalSection.Enter();
(gdb) p CriticalSection
$1 = {<NWindows::NSynchronization::Uncopyable> = {<No data fields>}, _object = {_mutex = {__data = {__lock = 2, __count = 0, __owner = 29376, __nusers = 1, __kind = 0, __spins = 0, __list = {
__prev = 0x0, __next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\300r\000\000\001", '\000' <repeats 26 times>, __align = 2}}}
(gdb) info thread
Id Target Id Frame
3 Thread 0x7fe9afe8d700 (LWP 29378) "7za" 0x00007fe9b513f8f4 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
2 Thread 0x7fe9af68c700 (LWP 29379) "7za" 0x00007fe9b513f8f4 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
* 1 Thread 0x7fe9b5537740 (LWP 29376) "7za" 0x00007fe9b514200c in __lll_lock_wait () from /lib/libpthread.so.0

I think the problem is in this code in CPP/7zip/Archive/Zip/ZipUpdate.cpp:
668 CMyComPtr<ISequentialInStream> fileInStream;
669 {

!! acuire lock here:
670 NWindows::NSynchronization::CCriticalSectionLock lock(mtProgressMixerSpec->Mixer2->CriticalSection);

671 HRESULT res = updateCallback->GetStream(ui.IndexInClient, &fileInStream);
672 if (res == S_FALSE)
673 {
674 complexity += ui.Size;
675 complexity += NFileHeader::kLocalBlockSize;

!! try to acquire the same lock here
676 mtProgressMixerSpec->Mixer2->SetProgressOffset(complexity);

677 RINOK(updateCallback->SetOperationResult(NArchive::NUpdate::NOperationResult::kOK));
678 refs.Refs[mtItemIndex - 1].Skip = true;
679 continue;
680 }
681 RINOK(res);
682 RINOK(updateCallback->SetOperationResult(NArchive::NUpdate::NOperationResult::kOK));
683 }

Backtrace:

Thread 3 (Thread 0x7fe9afe8d700 (LWP 29378)):
#0 0x00007fe9b513f8f4 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
#1 0x00000000004dc077 in Event_Wait (p=0x118dc90) at ../../../../C/Threads.c:492
#2 0x00000000004ab046 in Lock (this=0x118dc90) at ../../../Windows/Synchronization.h:64
#3 NArchive::NZip::CThreadInfo::WaitAndCode (this=0x118dc80) at ../../Archive/Zip/ZipUpdate.cpp:201
#4 0x00000000004ab0de in NArchive::NZip::CoderThread (threadCoderInfo=<optimized out>) at ../../Archive/Zip/ZipUpdate.cpp:217
#5 0x00007fe9b513be0e in start_thread () from /lib/libpthread.so.0
#6 0x00007fe9b46691ed in clone () from /lib/libc.so.6

Thread 2 (Thread 0x7fe9af68c700 (LWP 29379)):
#0 0x00007fe9b513f8f4 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/libpthread.so.0
#1 0x00000000004dc077 in Event_Wait (p=0x118ded0) at ../../../../C/Threads.c:492
#2 0x00000000004ab046 in Lock (this=0x118ded0) at ../../../Windows/Synchronization.h:64
#3 NArchive::NZip::CThreadInfo::WaitAndCode (this=0x118dec0) at ../../Archive/Zip/ZipUpdate.cpp:201
#4 0x00000000004ab0de in NArchive::NZip::CoderThread (threadCoderInfo=<optimized out>) at ../../Archive/Zip/ZipUpdate.cpp:217
#5 0x00007fe9b513be0e in start_thread () from /lib/libpthread.so.0
#6 0x00007fe9b46691ed in clone () from /lib/libc.so.6

Thread 1 (Thread 0x7fe9b5537740 (LWP 29376)):
#0 0x00007fe9b514200c in __lll_lock_wait () from /lib/libpthread.so.0
#1 0x00007fe9b513decc in _L_lock_513 () from /lib/libpthread.so.0
#2 0x00007fe9b513dd1b in pthread_mutex_lock () from /lib/libpthread.so.0
#3 0x00000000004ab19b in Enter (this=0x117afb0) at ../../../Windows/Synchronization.h:167
#4 NArchive::NZip::CMtProgressMixer2::SetProgressOffset (this=0x117af60, progressOffset=853) at ../../Archive/Zip/ZipUpdate.cpp:282
#5 0x00000000004aec24 in NArchive::NZip::Update2 (archive=..., inArchive=inArchive@entry=0x0, inStream=inStream@entry=0x0, inputItems=..., updateItems=..., options=options@entry=0x7fffec387580,
comment=0x0, updateCallback=updateCallback@entry=0x1193bc0) at ../../Archive/Zip/ZipUpdate.cpp:676
#6 0x00000000004b01c0 in NArchive::NZip::Update (inputItems=..., updateItems=..., seqOutStream=<optimized out>, inArchive=0x0, compressionMethodMode=0x7fffec387580, updateCallback=0x1193bc0)
at ../../Archive/Zip/ZipUpdate.cpp:1073
#7 0x00000000004a2301 in NArchive::NZip::CHandler::UpdateItems (this=0x118bae0, outStream=0x118bbf0, numItems=25, callback=0x1193bc0) at ../../Archive/Zip/ZipHandlerOut.cpp:376
#8 0x000000000044f32d in Compress (codecs=codecs@entry=0x1169120, actionSet=..., archive=archive@entry=0x0, compressionMethod=..., archivePath=..., arcItems=..., shareForWrite=false,
stdInMode=false, stdOutMode=false, dirItems=..., sfxMode=false, sfxModule=..., volumesSizes=..., tempFiles=..., errorInfo=..., callback=callback@entry=0x7fffec387d30)
at ../../UI/Common/Update.cpp:505
#9 0x000000000044fa13 in UpdateWithItemLists (codecs=codecs@entry=0x1169120, options=..., archive=0x0, arcItems=..., dirItems=..., tempFiles=..., errorInfo=...,
callback=callback@entry=0x7fffec387d30) at ../../UI/Common/Update.cpp:589
#10 0x0000000000451c15 in UpdateArchive (codecs=0x1169120, censor=..., options=..., errorInfo=..., openCallback=<optimized out>, callback=0x7fffec387d30) at ../../UI/Common/Update.cpp:827
#11 0x000000000040a680 in Main2 (numArgs=<optimized out>, args=<optimized out>) at ../../UI/Console/Main.cpp:538
#12 0x000000000040dbda in main (numArgs=5, args=0x7fffec388238) at ../../UI/Console/MainAr.cpp:55

[Anonymous]

A patch that fixes the problem -- don't try to acquire the lock in SetProgressOffset

[Anonymous]

Script to reproduce the bug. Run inside the source tree after bin/7za is builto

[Anonymous]

Core file

[Roman]

Bug still not fixed. As I saw, code in that place has not been changed since 4.6 version, and bug still exists.

Fix require some lines of code in one file. I have attached it. Maybe, it will be usefull for someone.
File: <root>/CPP/7zip/Archive/Zip/ZipUpdate.cpp</root>

[Igor Pavlov]

Is it bug of CCriticalSection / EnterCriticalSection() code in p7zip?
Does 7-Zip via wine work correctly in same cases?

[Roman]

Yes, looks like it is a bug with locking already locked non-recursive mutex on UNIX. Issue occurs only on Linux and only if some files has been deleted during achivation.
Issue can be easily reprodused. I know nothing about this issue in 7-zip. Also, I 'm receiving such deadlock only for ZIP type..

[Roman]