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.
This commit is contained in:
Adrien Destugues 2014-12-22 11:26:23 +01:00
parent a2b422eff9
commit 4227495829

View File

@ -94,58 +94,59 @@ bool
BLocker::Lock() BLocker::Lock()
{ {
status_t result; status_t result;
return AcquireLock(B_INFINITE_TIMEOUT, &result); return AcquireLock(B_INFINITE_TIMEOUT, &result);
} }
status_t status_t
BLocker::LockWithTimeout(bigtime_t timeout) BLocker::LockWithTimeout(bigtime_t timeout)
{ {
status_t result; status_t result;
AcquireLock(timeout, &result); AcquireLock(timeout, &result);
return result; return result;
} }
void void
BLocker::Unlock() BLocker::Unlock()
{ {
// If the thread currently holds the lockdecrement // The Be Book explicitly allows any thread, not just the lock owner, to
if (IsLocked()) { // unlock. This is bad practise and Haiku should not allow it.
// Decrement the number of outstanding locks this thread holds if (!IsLocked())
// on this BLocker. debugger("Trying to unlock from the wrong thread (#6400)");
fRecursiveCount--;
// If the recursive count is now at 0, that means the BLocker has // Decrement the number of outstanding locks this thread holds
// been released by the thread. // on this BLocker.
if (fRecursiveCount == 0) { fRecursiveCount--;
// The BLocker is no longer owned by any thread.
fLockOwner = B_ERROR;
// Decrement the benaphore count and store the undecremented // If the recursive count is now at 0, that means the BLocker has
// value in oldBenaphoreCount. // been released by the thread.
int32 oldBenaphoreCount = atomic_add(&fBenaphoreCount, -1); if (fRecursiveCount == 0) {
// The BLocker is no longer owned by any thread.
fLockOwner = B_ERROR;
// If the oldBenaphoreCount is greater than 1, then there is // Decrement the benaphore count and store the undecremented
// at lease one thread waiting for the lock in the case of a // value in oldBenaphoreCount.
// benaphore. int32 oldBenaphoreCount = atomic_add(&fBenaphoreCount, -1);
if (oldBenaphoreCount > 1) {
// Since there are threads waiting for the lock, it must // If the oldBenaphoreCount is greater than 1, then there is
// be released. Note, the old benaphore count will always be // at lease one thread waiting for the lock in the case of a
// greater than 1 for a semaphore so the release is always done. // benaphore.
release_sem(fSemaphoreID); 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 thread_id
BLocker::LockingThread() const 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. // This member returns true if the calling thread holds the lock.
// The easiest way to determine this is to compare the result of // The easiest way to determine this is to compare the result of
// find_thread() to the fLockOwner. // find_thread() to the fLockOwner.
return find_thread(NULL) == fLockOwner; return find_thread(NULL) == fLockOwner;
} }
int32 int32
BLocker::CountLocks() const BLocker::CountLocks() const
{ {
return fRecursiveCount; return fRecursiveCount;
} }
int32 int32
BLocker::CountLockRequests() const BLocker::CountLockRequests() const
{ {
return fBenaphoreCount; return fBenaphoreCount;
} }
sem_id sem_id
BLocker::Sem() const 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. // Only try to acquire the lock if the thread doesn't already own it.
if (!IsLocked()) { if (!IsLocked()) {
// Increment the benaphore count and test to see if it was already greater // Increment the benaphore count and test to see if it was already
// than 0. If it is greater than 0, then some thread already has the // greater than 0. If it is greater than 0, then some thread already has
// benaphore or the style is a semaphore. Either way, we need to acquire // the benaphore or the style is a semaphore. Either way, we need to
// the semaphore in this case. // acquire the semaphore in this case.
int32 oldBenaphoreCount = atomic_add(&fBenaphoreCount, 1); int32 oldBenaphoreCount = atomic_add(&fBenaphoreCount, 1);
if (oldBenaphoreCount > 0) { if (oldBenaphoreCount > 0) {
do { do {