From 1cffe0dc510ff62b37dc009694b256885fdcd6fb Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Tue, 3 Nov 2015 22:59:37 +0100 Subject: [PATCH] BLocker: Fix recursive count for "unusual" use cases. Since a BLocker can be unlocked from other threads than the one holding the lock, it can also be further unlocked even when already unlocked. This caused the recursive count to become negative. The first lock then needs to reinitialize the count to 1 for the lock balance to work again. Just incrementing the negative recursive count lead to it never counting back down from one to zero in the unlock case, which made the BLocker impossible to unlock. This makes the Haiku BLocker behave exactly like the BeOS one, including the negative recursive count and reinitialization, as evidenced by its debugging features showing the internal counts. Alternatively to reinitializing the recursive count it could be prevented from going below zero in the first place, but I don't see why we should deviate from BeOS there while allowing its awkward unlock behaviour. This makes some more exotic use cases work like the BGLView <-> SDL combination that previously would always just hang. While these abuses should be reviewed/corrected, just hanging the BLocker doesn't seem useful. --- src/kits/support/Locker.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/kits/support/Locker.cpp b/src/kits/support/Locker.cpp index 02a1bd83b5..0d4b672b34 100644 --- a/src/kits/support/Locker.cpp +++ b/src/kits/support/Locker.cpp @@ -279,8 +279,11 @@ BLocker::AcquireLock(bigtime_t timeout, status_t *error) // Set the lock owner to this thread and increment the recursive count // by one. The recursive count is incremented because one more Unlock() // is now required to release the lock (ie, 0 => 1, 1 => 2 etc). - fLockOwner = find_thread(NULL); - fRecursiveCount++; + if (fLockOwner < 0) { + fLockOwner = find_thread(NULL); + fRecursiveCount = 1; + } else + fRecursiveCount++; } if (error != NULL)