Fix hangs during pbzip2 compression.
pbzip2 sometimes hangs during compression because it doesn't lock when
calling pthread_cond_broadcast(..), so the thread holding the lock might
not get the message if it has already checked the condition but has not
called pthread_cond_wait(...) yet. To fix this, I've upgraded pbzip2 to
consistently lock before calling pthread_cond_* functions.
These hangs can be reproduced consistently if you add a 5 second sleep to
safe_cond_wait(...) before it calls pthread_cond_wait(...). With my fix,
the hangs no longer occur.
This bug is tracked upstream at https://bugs.launchpad.net/pbzip2/+bug/876686
I've also upgraded to pbzip2 1.1.5.
BUG=chromium-os:21726
TEST=Test compressing any file (e.g. an empty file) with the 5 second sleep.
Before the change, it hangs every time. After the change, it doesn't
hang. Also run trybot run.
Reviewed-on: http://gerrit.chromium.org/gerrit/10164
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Commit-Ready: David James <davidjames@chromium.org>
Tested-by: David James <davidjames@chromium.org>
Reviewed-by: Zdenek Behan <zbehan@chromium.org>
(cherry picked from commit 5b47f7a0729e0764981a3d4dd7ae95d09fd373b0)
Change-Id: I7e6a22aa1146aa37ef5c9c899a9f4da788096add
Reviewed-on: http://gerrit.chromium.org/gerrit/10274
Reviewed-by: Scott Zawalski <scottz@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Commit-Ready: David James <davidjames@chromium.org>
Tested-by: David James <davidjames@chromium.org>
diff --git a/app-arch/pbzip2/files/pbzip2-1.1.5-hang-fix.patch b/app-arch/pbzip2/files/pbzip2-1.1.5-hang-fix.patch
new file mode 100644
index 0000000..429e1f9
--- /dev/null
+++ b/app-arch/pbzip2/files/pbzip2-1.1.5-hang-fix.patch
@@ -0,0 +1,125 @@
+--- pbzip2.cpp 2011-07-16 09:52:35.000000000 -0700
++++ pbzip2-fixed.cpp 2011-10-17 10:23:02.626471000 -0700
+@@ -340,7 +340,6 @@
+ inline int syncGetTerminateFlag();
+ inline void syncSetTerminateFlag(int newValue);
+ inline void syncSetFinishedFlag(int newValue);
+-inline void setLastGoodBlock(int newValue);
+ inline void syncSetLastGoodBlock(int newValue);
+ inline int syncGetLastGoodBlock();
+ void cleanupUnfinishedWork();
+@@ -668,17 +667,24 @@
+ {
+ // wake up terminator thread
+ pthread_cond_signal(&TerminateCond);
++ safe_mutex_unlock(&TerminateFlagMutex);
+
+ // wake up all other possibly blocked on cond threads
++ safe_mutex_lock(OutMutex);
+ pthread_cond_broadcast(notTooMuchNumBuffered);
++ safe_mutex_unlock(OutMutex);
+ if (FifoQueue != NULL)
+ {
++ safe_mutex_lock(FifoQueue->mut);
+ pthread_cond_broadcast(FifoQueue->notFull);
+ pthread_cond_broadcast(FifoQueue->notEmpty);
++ safe_mutex_unlock(FifoQueue->mut);
+ }
+ }
+-
+- safe_mutex_unlock(&TerminateFlagMutex);
++ else
++ {
++ safe_mutex_unlock(&TerminateFlagMutex);
++ }
+ }
+
+ /*
+@@ -697,11 +703,12 @@
+ safe_mutex_unlock(&TerminateFlagMutex);
+ }
+
+-inline void setLastGoodBlock(int newValue)
++inline void syncSetLastGoodBlock(int newValue)
+ {
++ safe_mutex_lock(OutMutex);
+ #ifdef PBZIP_DEBUG
+ unsigned long long thid = (unsigned long long) pthread_self();
+- fprintf(stderr, "(%"PRIu64") setLastGoodBlock: %d -> %d\n", thid, LastGoodBlock, newValue );
++ fprintf(stderr, "(%"PRIu64") syncSetLastGoodBlock: %d -> %d\n", thid, LastGoodBlock, newValue );
+ #endif
+
+ if ( (LastGoodBlock == -1) || (newValue < LastGoodBlock) )
+@@ -713,19 +720,20 @@
+
+ // wake up all other possibly blocked on cond threads
+ pthread_cond_broadcast(notTooMuchNumBuffered);
++ safe_mutex_unlock(OutMutex);
++
+ if (FifoQueue != NULL)
+ {
++ safe_mutex_lock(FifoQueue->mut);
+ pthread_cond_broadcast(FifoQueue->notFull);
+ pthread_cond_broadcast(FifoQueue->notEmpty);
++ safe_mutex_unlock(FifoQueue->mut);
+ }
+ }
+-}
+-
+-inline void syncSetLastGoodBlock(int newValue)
+-{
+- safe_mutex_lock(OutMutex);
+- setLastGoodBlock(newValue);
+- safe_mutex_unlock(OutMutex);
++ else
++ {
++ safe_mutex_unlock(OutMutex);
++ }
+ }
+
+ inline int syncGetLastGoodBlock()
+@@ -1299,7 +1307,9 @@
+
+ if (producerDecompressCheckInterrupt(hInfile, fileData, NumBlocks) != 0)
+ {
++ safe_mutex_lock(fifo->mut);
+ safe_cond_broadcast(fifo->notEmpty); // just in case
++ safe_mutex_unlock(fifo->mut);
+ syncSetProducerDone(1);
+ return 0;
+ }
+@@ -1368,7 +1378,9 @@
+ }
+
+ syncSetProducerDone(1);
++ safe_mutex_lock(fifo->mut);
+ safe_cond_broadcast(fifo->notEmpty); // just in case
++ safe_mutex_unlock(fifo->mut);
+
+ #ifdef PBZIP_DEBUG
+ fprintf(stderr, "producer: Done - exiting. Last Block: %d\n", NumBlocks);
+@@ -1918,10 +1930,14 @@
+ // wake up all other possibly blocked on cond threads
+ if (FifoQueue != NULL)
+ {
++ safe_mutex_lock(FifoQueue->mut);
+ safe_cond_broadcast(FifoQueue->notEmpty); // important
+ safe_cond_broadcast(FifoQueue->notFull); // not really needed
++ safe_mutex_unlock(FifoQueue->mut);
+ }
++ safe_mutex_lock(OutMutex);
+ safe_cond_broadcast(notTooMuchNumBuffered); // not really needed
++ safe_mutex_unlock(OutMutex);
+
+ if (QuietMode != 1)
+ {
+@@ -2559,7 +2575,9 @@
+ close(hInfile);
+
+ syncSetProducerDone(1);
++ safe_mutex_lock(fifo->mut);
+ safe_cond_broadcast(fifo->notEmpty); // just in case
++ safe_mutex_unlock(fifo->mut);
+
+ #ifdef PBZIP_DEBUG
+ fprintf(stderr, "producer: Done - exiting. Num Blocks: %d\n", NumBlocks);
diff --git a/app-arch/pbzip2/files/pbzip2-hang.patch b/app-arch/pbzip2/files/pbzip2-hang.patch
deleted file mode 100644
index 8a5db3a..0000000
--- a/app-arch/pbzip2/files/pbzip2-hang.patch
+++ /dev/null
@@ -1,41 +0,0 @@
-=== modified file 'pbzip2.cpp'
---- pbzip2.cpp 2011-04-01 20:13:21 +0000
-+++ pbzip2.cpp 2011-04-16 12:27:45 +0000
-@@ -192,6 +192,8 @@
- * - Default extension on decompress of .tbz2 changed to .tar for
- * bzip2 compatibility (bug #743639)
- * - Print trailing garbage errors even when in quiet mode (bug #743635)
-+ * - Fixed hang on decompress with --ignore-trailing-garbage=1 when
-+ * producer is interrupted on trailing garbage (bug #762464)
- *
- *
- * Specials thanks for suggestions and testing: Phillippe Welsh,
-@@ -1247,6 +1249,8 @@
-
- if (producerDecompressCheckInterrupt(hInfile, fileData, NumBlocks) != 0)
- {
-+ safe_cond_broadcast(fifo->notEmpty); // just in case
-+ syncSetProducerDone(1);
- return 0;
- }
-
-@@ -1272,6 +1276,8 @@
-
- if (producerDecompressCheckInterrupt(hInfile, fileData, NumBlocks) != 0)
- {
-+ safe_cond_broadcast(fifo->notEmpty); // just in case
-+ syncSetProducerDone(1);
- safe_mutex_unlock(fifo->mut);
- return 0;
- }
-@@ -1344,7 +1350,8 @@
- #endif
- }
- if ( (syncGetLastGoodBlock() != -1) &&
-- ( (lastElement == NULL) || (lastElement->blockNumber > syncGetLastGoodBlock()) ) )
-+ ( (lastElement == NULL) || (lastElement->blockNumber > syncGetLastGoodBlock())
-+ || lastElement->isLastInSequence ) )
- {
- isInterrupted = true;
-
-
diff --git a/app-arch/pbzip2/pbzip2-1.1.3.ebuild b/app-arch/pbzip2/pbzip2-1.1.5.ebuild
similarity index 95%
rename from app-arch/pbzip2/pbzip2-1.1.3.ebuild
rename to app-arch/pbzip2/pbzip2-1.1.5.ebuild
index 13fdd1d..c5eaf1c 100644
--- a/app-arch/pbzip2/pbzip2-1.1.3.ebuild
+++ b/app-arch/pbzip2/pbzip2-1.1.5.ebuild
@@ -19,7 +19,7 @@
src_unpack() {
unpack ${A}
cd ${S}
- epatch "${FILESDIR}"/pbzip2-hang.patch || die
+ epatch "${FILESDIR}"/${P}-hang-fix.patch || die
sed -e 's:^CFLAGS = .*$:#&:g' -e 's:g++:$(CXX):g' -i Makefile || die
}
