[haiku-commits] haiku: hrev48542 - src/kits/support
- From: pulkomandy@xxxxxxxxxxxxx
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Mon, 22 Dec 2014 11:28:45 +0100 (CET)
hrev48542 adds 1 changeset to branch 'master'
old head: a2b422eff9bf2cd396ebd31630632c3224c539b5
new head: 42274958297907618f9ac3df199b35dd3399d114
overview: http://cgit.haiku-os.org/haiku/log/?qt=range&q=4227495+%5Ea2b422e
----------------------------------------------------------------------------
4227495: BLocker: call debugger() when unlocking from another thread
BeOS did allow other threads than the owner to unlock a BLocker (the be
book says so). We did not, and silently ignored the unlock attempt in
this case, probably resulting in a deadlock of applications using the
feature.
Call debugger instead so:
* The problem is made visible for such apps
* The debugger call is continuable so the app can be run, still
Will help making a decision on what to do here (follow BeOS or change
behavior) and make a final decision for #6400.
[ Adrien Destugues <pulkomandy@xxxxxxxxx> ]
----------------------------------------------------------------------------
Revision: hrev48542
Commit: 42274958297907618f9ac3df199b35dd3399d114
URL: http://cgit.haiku-os.org/haiku/commit/?id=4227495
Author: Adrien Destugues <pulkomandy@xxxxxxxxx>
Date: Mon Dec 22 10:26:23 2014 UTC
Ticket: https://dev.haiku-os.org/ticket/6400
----------------------------------------------------------------------------
1 file changed, 41 insertions(+), 40 deletions(-)
src/kits/support/Locker.cpp | 81 +++++++++++++++++++++--------------------
----------------------------------------------------------------------------
diff --git a/src/kits/support/Locker.cpp b/src/kits/support/Locker.cpp
index 630c22e..63faadb 100644
--- a/src/kits/support/Locker.cpp
+++ b/src/kits/support/Locker.cpp
@@ -94,58 +94,59 @@ bool
BLocker::Lock()
{
status_t result;
- return AcquireLock(B_INFINITE_TIMEOUT, &result);
+ return AcquireLock(B_INFINITE_TIMEOUT, &result);
}
status_t
BLocker::LockWithTimeout(bigtime_t timeout)
{
- status_t result;
+ status_t result;
AcquireLock(timeout, &result);
- return result;
+ return result;
}
-
void
BLocker::Unlock()
{
- // If the thread currently holds the lockdecrement
- if (IsLocked()) {
- // Decrement the number of outstanding locks this thread holds
- // on this BLocker.
- fRecursiveCount--;
-
- // If the recursive count is now at 0, that means the BLocker
has
- // been released by the thread.
- if (fRecursiveCount == 0) {
- // The BLocker is no longer owned by any thread.
- fLockOwner = B_ERROR;
-
- // Decrement the benaphore count and store the undecremented
- // value in oldBenaphoreCount.
- int32 oldBenaphoreCount = atomic_add(&fBenaphoreCount,
-1);
-
- // If the oldBenaphoreCount is greater than 1, then
there is
- // at lease one thread waiting for the lock in the case
of a
- // benaphore.
- if (oldBenaphoreCount > 1) {
- // Since there are threads waiting for the
lock, it must
- // be released. Note, the old benaphore count
will always be
- // greater than 1 for a semaphore so the
release is always done.
- release_sem(fSemaphoreID);
- }
- }
- }
+ // The Be Book explicitly allows any thread, not just the lock owner, to
+ // unlock. This is bad practise and Haiku should not allow it.
+ if (!IsLocked())
+ debugger("Trying to unlock from the wrong thread (#6400)");
+
+ // Decrement the number of outstanding locks this thread holds
+ // on this BLocker.
+ fRecursiveCount--;
+
+ // If the recursive count is now at 0, that means the BLocker has
+ // been released by the thread.
+ if (fRecursiveCount == 0) {
+ // The BLocker is no longer owned by any thread.
+ fLockOwner = B_ERROR;
+
+ // Decrement the benaphore count and store the undecremented
+ // value in oldBenaphoreCount.
+ int32 oldBenaphoreCount = atomic_add(&fBenaphoreCount, -1);
+
+ // If the oldBenaphoreCount is greater than 1, then there is
+ // at lease one thread waiting for the lock in the case of a
+ // benaphore.
+ if (oldBenaphoreCount > 1) {
+ // Since there are threads waiting for the lock, it must
+ // be released. Note, the old benaphore count will
always be
+ // greater than 1 for a semaphore so the release is
always done.
+ release_sem(fSemaphoreID);
+ }
+ }
}
thread_id
BLocker::LockingThread() const
{
- return fLockOwner;
+ return fLockOwner;
}
@@ -155,28 +156,28 @@ BLocker::IsLocked() const
// This member returns true if the calling thread holds the lock.
// The easiest way to determine this is to compare the result of
// find_thread() to the fLockOwner.
- return find_thread(NULL) == fLockOwner;
+ return find_thread(NULL) == fLockOwner;
}
int32
BLocker::CountLocks() const
{
- return fRecursiveCount;
+ return fRecursiveCount;
}
int32
BLocker::CountLockRequests() const
{
- return fBenaphoreCount;
+ return fBenaphoreCount;
}
sem_id
BLocker::Sem() const
{
- return fSemaphoreID;
+ return fSemaphoreID;
}
@@ -217,10 +218,10 @@ BLocker::AcquireLock(bigtime_t timeout, status_t *error)
// Only try to acquire the lock if the thread doesn't already own it.
if (!IsLocked()) {
- // Increment the benaphore count and test to see if it was
already greater
- // than 0. If it is greater than 0, then some thread already
has the
- // benaphore or the style is a semaphore. Either way, we need
to acquire
- // the semaphore in this case.
+ // Increment the benaphore count and test to see if it was
already
+ // greater than 0. If it is greater than 0, then some thread
already has
+ // the benaphore or the style is a semaphore. Either way, we
need to
+ // acquire the semaphore in this case.
int32 oldBenaphoreCount = atomic_add(&fBenaphoreCount, 1);
if (oldBenaphoreCount > 0) {
do {
Other related posts:
