i2p/haiku-beta6
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

* Replaced DEBUG MultiLocker with an implementation that actually helps debugging

Browse Source 
locking problems (instead of debugging the locker class).
* MultiLocker::IsReadLocked() is now only exported with DEBUG mode turned on, as
  it only works correctly in this case.
* Made MultiLocker safe against B_INTERRUPTED, ie. it now just tries to lock again
  instead of failing for no obvious reason.
* Removed bogus arguments to acquire_sem_etc() in MultiLocker (like B_DO_NOT_RESCHEDULE).
* Applied coding style to MultiLocker.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@20055 a95241bf-73f2-0310-859d-f6bbb57e9c96
This commit is contained in:
Axel Dörfler
2007-02-02 19:16:18 +00:00
parent cb134e2550
commit 4d1fd46cdf
4 changed files with 534 additions and 442 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/servers/app/DesktopSettings.cpp
+4 -1
View File
@@ -456,8 +456,11 @@ DesktopSettings::DesktopSettings(Desktop* desktop)
:
fSettings(desktop->fSettings)
{
if (!desktop->fWindowLock.IsReadLocked() && !desktop->fWindowLock.IsWriteLocked())
#if DEBUG
if (!desktop->fWindowLock.IsWriteLocked()
&& !desktop->fWindowLock.IsReadLocked())
debugger("desktop not locked when trying to access settings");
#endif
}
src/servers/app/MultiLocker.cpp
+267 -196
View File
@@ -1,41 +1,50 @@
/* MultiLocker.cpp */
/*
Copyright 1999, Be Incorporated. All Rights Reserved.
This file may be used under the terms of the Be Sample Code License.
* Copyright 2005-2007, Haiku, Inc. All Rights Reserved.
* Distributed under the terms of the MIT license.
*
* Copyright 1999, Be Incorporated. All Rights Reserved.
* This file may be used under the terms of the Be Sample Code License.
*/
#include "MultiLocker.h"
#include <Debug.h>
#include <Errors.h>
#include <OS.h>
//#define TIMING 1
//#define DEBUG 1
#define TIMING MULTI_LOCKER_TIMING
#define DEBUG MULTI_LOCKER_DEBUG
MultiLocker::MultiLocker(const char* semaphoreBaseName)
: fInit(B_NO_INIT),
const int32 LARGE_NUMBER = 100000;
MultiLocker::MultiLocker(const char* baseName)
:
#if DEBUG
fDebugArray(NULL),
fMaxThreads(0),
#else
fReadCount(0),
fReadSem(-1),
fWriteCount(0),
fWriteSem(-1),
fLockCount(0),
fWriterLock(-1),
#endif
fInit(B_NO_INIT),
fWriterNest(0),
fWriterThread(-1),
fWriterStackBase(0),
fDebugArray(NULL),
fMaxThreads(0)
fWriterStackBase(0)
{
// build the semaphores
if (semaphoreBaseName) {
#if !DEBUG
if (baseName) {
char name[128];
sprintf(name, "%s-%s", semaphoreBaseName, "ReadSem");
sprintf(name, "%s-%s", baseName, "ReadSem");
fReadSem = create_sem(0, name);
sprintf(name, "%s-%s", semaphoreBaseName, "WriteSem");
sprintf(name, "%s-%s", baseName, "WriteSem");
fWriteSem = create_sem(0, name);
sprintf(name, "%s-%s", semaphoreBaseName, "WriterLock");
sprintf(name, "%s-%s", baseName, "WriterLock");
fWriterLock = create_sem(0, name);
} else {
fReadSem = create_sem(0, "MultiLocker_ReadSem");
@@ -45,8 +54,11 @@ MultiLocker::MultiLocker(const char* semaphoreBaseName)
if (fReadSem >= 0 && fWriteSem >=0 && fWriterLock >= 0)
fInit = B_OK;
#else
fLock = create_sem(LARGE_NUMBER, baseName != NULL ? baseName : "MultiLocker");
if (fLock >= 0)
fInit = B_OK;
#if DEBUG
// we are in debug mode!
// create the reader tracking list
// the array needs to be large enough to hold all possible threads
@@ -57,7 +69,6 @@ MultiLocker::MultiLocker(const char* semaphoreBaseName)
for (int32 i = 0; i < fMaxThreads; i++) {
fDebugArray[i] = 0;
}
#endif
#if TIMING
//initialize the counter variables
@@ -74,19 +85,19 @@ MultiLocker::MultiLocker(const char* semaphoreBaseName)
MultiLocker::~MultiLocker()
{
// become the writer
if (!IsWriteLocked()) WriteLock();
if (!IsWriteLocked())
WriteLock();
// set locker to be uninitialized
fInit = B_NO_INIT;
#if !DEBUG
// delete the semaphores
delete_sem(fReadSem);
delete_sem(fWriteSem);
delete_sem(fWriterLock);
#if DEBUG
//we are in debug mode!
//clear and delete the reader tracking list
#else
delete_sem(fLock);
free(fDebugArray);
#endif
#if TIMING
@@ -101,24 +112,89 @@ MultiLocker::~MultiLocker()
ru_count > 0 ? ru_time / ru_count : 0,
wl_count > 0 ? wl_time / wl_count : 0,
wu_count > 0 ? wu_time / wu_count : 0,
islock_count > 0 ? islock_time / islock_count : 0
);
#if DEBUG
printf( "Avg register_thread: %lld\n"
"Avg unregister_thread: %lld\n",
reg_count > 0 ? reg_time / reg_count : 0,
unreg_count > 0 ? unreg_time / unreg_count : 0
);
#endif
islock_count > 0 ? islock_time / islock_count : 0);
#endif
}
status_t
MultiLocker::InitCheck()
{
return fInit;
}
/*!
This function demonstrates a nice method of determining if the current thread
is the writer or not. The method involves caching the index of the page in memory
where the thread's stack is located. Each time a new writer acquires the lock,
its thread_id and stack_page are recorded. IsWriteLocked gets the stack_page of the
current thread and sees if it is a match. If the stack_page matches you are guaranteed
to have the matching thread. If the stack page doesn't match the more traditional
find_thread(NULL) method of matching the thread_ids is used.
This technique is very useful when dealing with a lock that is acquired in a nested fashion.
It could be expanded to cache the information of the last thread in the lock, and then if
the same thread returns while there is no one in the lock, it could save some time, if the
same thread is likely to acquire the lock again and again.
I should note another shortcut that could be implemented here
If fWriterThread is set to -1 then there is no writer in the lock, and we could
return from this function much faster. However the function is currently set up
so all of the stack_base and thread_id info is determined here. WriteLock passes
in some variables so that if the lock is not held it does not have to get the thread_id
and stack base again. Instead this function returns that information. So this shortcut
would only move this information gathering outside of this function, and I like it all
contained.
*/
bool
MultiLocker::IsWriteLocked(uint32* _stackBase, thread_id* _thread)
{
#if TIMING
bigtime_t start = system_time();
#endif
// get a variable on the stack
bool writeLockHolder = false;
if (fInit == B_OK) {
// determine which page in memory this stack represents
// this is managed by taking the address of the item on the
// stack and dividing it by the size of the memory pages
// if it is the same as the cached stack_page, there is a match
uint32 stackBase = (uint32)&writeLockHolder / B_PAGE_SIZE;
thread_id thread = 0;
if (fWriterStackBase == stackBase) {
writeLockHolder = true;
} else {
// as there was no stack page match we resort to the
// tried and true methods
thread = find_thread(NULL);
if (fWriterThread == thread)
writeLockHolder = true;
}
// if someone wants this information, give it to them
if (_stackBase != NULL)
*_stackBase = stackBase;
if (_thread != NULL)
*_thread = thread;
}
#if TIMING
bigtime_t end = system_time();
islock_time += (end - start);
islock_count++;
#endif
return writeLockHolder;
}
#if !DEBUG
// #pragma mark - Standard versions
bool
MultiLocker::ReadLock()
{
@@ -132,27 +208,22 @@ MultiLocker::ReadLock()
if (fInit == B_OK) {
if (IsWriteLocked()) {
// the writer simply increments the nesting
#if DEBUG
if (fWriterNest < 0)
debugger("ReadLock() - negative writer nest count\n");
#endif
fWriterNest++;
locked = true;
} else {
// increment and retrieve the current count of readers
int32 current_count = atomic_add(&fReadCount, 1);
if (current_count < 0) {
int32 currentCount = atomic_add(&fReadCount, 1);
if (currentCount < 0) {
// a writer holds the lock so wait for fReadSem to be released
locked = (acquire_sem_etc(fReadSem, 1, B_DO_NOT_RESCHEDULE,
B_INFINITE_TIMEOUT) == B_OK);
} else locked = true;
#if DEBUG
//register if we acquired the lock
if (locked) register_thread();
#endif
}
status_t status;
do {
status = acquire_sem(fReadSem);
} while (status == B_INTERRUPTED);
locked = status == B_OK;
} else
locked = true;
}
}
#if TIMING
@@ -164,6 +235,7 @@ MultiLocker::ReadLock()
return locked;
}
bool
MultiLocker::WriteLock()
{
@@ -174,32 +246,25 @@ MultiLocker::WriteLock()
bool locked = false;
if (fInit == B_OK) {
uint32 stack_base = 0;
uint32 stackBase = 0;
thread_id thread = -1;
if (IsWriteLocked(&stack_base, &thread)) {
if (IsWriteLocked(&stackBase, &thread)) {
// already the writer - increment the nesting count
#if DEBUG
if (fWriterNest < 0)
debugger("WriteLock() - negative nest count\n");
#endif
fWriterNest++;
locked = true;
} else {
// new writer acquiring the lock
#if DEBUG
// NOTE: IsReadLocked() tells you
// if this thread really holds the
// "read lock" only in DEBUG mode!
if (IsReadLocked())
debugger("Reader wants to become writer!");
#endif
if (atomic_add(&fLockCount, 1) >= 1) {
// another writer in the lock - acquire the semaphore
locked = (acquire_sem_etc(fWriterLock, 1, B_DO_NOT_RESCHEDULE,
B_INFINITE_TIMEOUT) == B_OK);
} else locked = true;
status_t status;
do {
status = acquire_sem(fWriterLock);
} while (status == B_INTERRUPTED);
locked = status == B_OK;
} else
locked = true;
if (locked) {
// new holder of the lock
@@ -207,18 +272,20 @@ MultiLocker::WriteLock()
// decrement fReadCount by a very large number
// this will cause new readers to block on fReadSem
int32 readers = atomic_add(&fReadCount, -LARGE_NUMBER);
if (readers > 0) {
// readers hold the lock - acquire fWriteSem
locked = (acquire_sem_etc(fWriteSem, readers,
B_DO_NOT_RESCHEDULE,
B_INFINITE_TIMEOUT) == B_OK);
status_t status;
do {
status = acquire_sem_etc(fWriteSem, readers, 0, 0);
} while (status == B_INTERRUPTED);
locked = status == B_OK;
}
if (locked) {
ASSERT(fWriterThread == -1);
// record thread information
fWriterThread = thread;
fWriterStackBase = stack_base;
fWriterStackBase = stackBase;
}
}
}
@@ -233,6 +300,7 @@ MultiLocker::WriteLock()
return locked;
}
bool
MultiLocker::ReadUnlock()
{
@@ -245,26 +313,15 @@ MultiLocker::ReadUnlock()
if (IsWriteLocked()) {
// writers simply decrement the nesting count
fWriterNest--;
#if DEBUG
if (fWriterNest < 0)
debugger("ReadUnlock() - negative writer nest count\n");
#endif
unlocked = true;
} else {
// decrement and retrieve the read counter
int32 current_count = atomic_add(&fReadCount, -1);
if (current_count < 0) {
// a writer is waiting for the lock so release fWriteSem
unlocked = (release_sem_etc(fWriteSem, 1,
B_DO_NOT_RESCHEDULE) == B_OK);
} else unlocked = true;
#if DEBUG
//unregister if we released the lock
if (unlocked) unregister_thread();
#endif
unlocked = release_sem_etc(fWriteSem, 1, B_DO_NOT_RESCHEDULE) == B_OK;
} else
unlocked = true;
}
#if TIMING
@@ -276,6 +333,7 @@ MultiLocker::ReadUnlock()
return unlocked;
}
bool
MultiLocker::WriteUnlock()
{
@@ -289,12 +347,6 @@ MultiLocker::WriteUnlock()
// if this is a nested lock simply decrement the nest count
if (fWriterNest > 0) {
fWriterNest--;
#if DEBUG
if (fWriterNest < 0)
debugger("WriteUnlock(): nest count now negative\n");
#endif
unlocked = true;
} else {
// writer finally unlocking
@@ -307,9 +359,10 @@ MultiLocker::WriteUnlock()
if (readersWaiting > 0) {
// readers are waiting to acquire the lock
unlocked = (release_sem_etc(fReadSem, readersWaiting,
B_DO_NOT_RESCHEDULE) == B_OK);
} else unlocked = true;
unlocked = release_sem_etc(fReadSem, readersWaiting,
B_DO_NOT_RESCHEDULE) == B_OK;
} else
unlocked = true;
if (unlocked) {
// clear the information
@@ -319,13 +372,13 @@ MultiLocker::WriteUnlock()
// decrement and retrieve the lock count
if (atomic_add(&fLockCount, -1) > 1) {
// other writers are waiting so release fWriterLock
unlocked = (release_sem_etc(fWriterLock, 1,
B_DO_NOT_RESCHEDULE) == B_OK);
unlocked = release_sem_etc(fWriterLock, 1,
B_DO_NOT_RESCHEDULE) == B_OK;
}
}
}
} else debugger("Non-writer attempting to WriteUnlock()\n");
} else
debugger("Non-writer attempting to WriteUnlock()");
#if TIMING
bigtime_t end = system_time();
@@ -336,92 +389,139 @@ MultiLocker::WriteUnlock()
return unlocked;
}
/* this function demonstrates a nice method of determining if the current thread */
/* is the writer or not. The method involves caching the index of the page in memory */
/* where the thread's stack is located. Each time a new writer acquires the lock, */
/* its thread_id and stack_page are recorded. IsWriteLocked gets the stack_page of the */
/* current thread and sees if it is a match. If the stack_page matches you are guaranteed */
/* to have the matching thread. If the stack page doesn't match the more traditional */
/* find_thread(NULL) method of matching the thread_ids is used. */
/* This technique is very useful when dealing with a lock that is acquired in a nested fashion. */
/* It could be expanded to cache the information of the last thread in the lock, and then if */
/* the same thread returns while there is no one in the lock, it could save some time, if the */
/* same thread is likely to acquire the lock again and again. */
/* I should note another shortcut that could be implemented here */
/* If fWriterThread is set to -1 then there is no writer in the lock, and we could */
/* return from this function much faster. However the function is currently set up */
/* so all of the stack_base and thread_id info is determined here. WriteLock passes */
/* in some variables so that if the lock is not held it does not have to get the thread_id */
/* and stack base again. Instead this function returns that information. So this shortcut */
/* would only move this information gathering outside of this function, and I like it all */
/* contained. */
#else // DEBUG
// #pragma mark - Debug versions
bool
MultiLocker::IsWriteLocked(uint32 *the_stack_base, thread_id *the_thread)
MultiLocker::ReadLock()
{
#if TIMING
bigtime_t start = system_time();
#endif
bool locked = false;
//get a variable on the stack
bool write_lock_holder = false;
if (fInit != B_OK)
debugger("lock not initialized");
if (fInit == B_OK) {
uint32 stack_base;
thread_id thread = 0;
if (IsWriteLocked()) {
if (fWriterNest < 0)
debugger("ReadLock() - negative writer nest count");
//determine which page in memory this stack represents
//this is managed by taking the address of the item on the
//stack and dividing it by the size of the memory pages
//if it is the same as the cached stack_page, there is a match
stack_base = (uint32)&write_lock_holder / B_PAGE_SIZE;
if (fWriterStackBase == stack_base) {
write_lock_holder = true;
fWriterNest++;
locked = true;
} else {
//as there was no stack_page match we resort to the
//tried and true methods
thread = find_thread(NULL);
if (fWriterThread == thread) {
write_lock_holder = true;
status_t status;
do {
status = acquire_sem(fLock);
} while (status == B_INTERRUPTED);
locked = status == B_OK;
if (locked)
_RegisterThread();
}
return locked;
}
bool
MultiLocker::WriteLock()
{
bool locked = false;
if (fInit != B_OK)
debugger("lock not initialized");
uint32 stackBase = 0;
thread_id thread = -1;
if (IsWriteLocked(&stackBase, &thread)) {
if (fWriterNest < 0)
debugger("WriteLock() - negative writer nest count");
fWriterNest++;
locked = true;
} else {
// new writer acquiring the lock
if (IsReadLocked())
debugger("Reader wants to become writer!");
status_t status;
do {
status = acquire_sem_etc(fLock, LARGE_NUMBER, 0, 0);
} while (status == B_INTERRUPTED);
locked = status == B_OK;
if (locked) {
// record thread information
fWriterThread = thread;
fWriterStackBase = stackBase;
}
}
//if someone wants this information, give it to them
if (the_stack_base != NULL) {
*the_stack_base = stack_base;
}
if (the_thread != NULL) {
*the_thread = thread;
}
return locked;
}
#if TIMING
bigtime_t end = system_time();
islock_time += (end - start);
islock_count++;
#endif
return write_lock_holder;
bool
MultiLocker::ReadUnlock()
{
bool unlocked = false;
if (IsWriteLocked()) {
// writers simply decrement the nesting count
fWriterNest--;
if (fWriterNest < 0)
debugger("ReadUnlock() - negative writer nest count");
unlocked = true;
} else {
// decrement and retrieve the read counter
unlocked = release_sem_etc(fLock, 1, B_DO_NOT_RESCHEDULE) == B_OK;
if (unlocked)
_UnregisterThread();
}
return unlocked;
}
bool
MultiLocker::WriteUnlock()
{
bool unlocked = false;
if (IsWriteLocked()) {
// if this is a nested lock simply decrement the nest count
if (fWriterNest > 0) {
fWriterNest--;
unlocked = true;
} else {
unlocked = release_sem_etc(fLock, LARGE_NUMBER, B_DO_NOT_RESCHEDULE) == B_OK;
if (unlocked) {
// clear the information
fWriterThread = -1;
fWriterStackBase = 0;
}
}
} else {
debug_printf("write holder %ld\n", fWriterThread);
debugger("Non-writer attempting to WriteUnlock()");
}
return unlocked;
}
bool
MultiLocker::IsReadLocked()
{
//a properly initialized MultiLocker in non-debug always returns true
bool locked = true;
if (fInit == B_NO_INIT) locked = false;
if (fInit == B_NO_INIT)
return false;
#if DEBUG
// determine if the lock is actually held
thread_id thread = find_thread(NULL);
if (fDebugArray[thread % fMaxThreads] > 0)
locked = true;
else
locked = false;
#endif
return locked;
return fDebugArray[thread % fMaxThreads] > 0;
}
@@ -447,53 +547,24 @@ MultiLocker::IsReadLocked()
/* was not deemed to be a problem */
void
MultiLocker::register_thread()
MultiLocker::_RegisterThread()
{
#if DEBUG
#if TIMING
bigtime_t start = system_time();
#endif
thread_id thread = find_thread(NULL);
if (fDebugArray[thread % fMaxThreads] != 0)
debugger("Nested ReadLock!\n");
debugger("Nested ReadLock!");
fDebugArray[thread % fMaxThreads]++;
#if TIMING
bigtime_t end = system_time();
reg_time += (end - start);
reg_count++;
#endif
#else
debugger("register_thread should never be called unless in DEBUG mode!\n");
#endif
}
void
MultiLocker::unregister_thread()
{
#if DEBUG
#if TIMING
bigtime_t start = system_time();
#endif
void
MultiLocker::_UnregisterThread()
{
thread_id thread = find_thread(NULL);
ASSERT(fDebugArray[thread % fMaxThreads] == 1);
fDebugArray[thread % fMaxThreads]--;
#if TIMING
bigtime_t end = system_time();
unreg_time += (end - start);
unreg_count++;
#endif
#else
debugger("unregister_thread should never be called unless in DEBUG mode!\n");
#endif
}
#endif // DEBUG
src/servers/app/MultiLocker.h
+47 -31
View File
@@ -1,13 +1,12 @@
/* MultiLocker.h */
/*
Copyright 2005-2006, Haiku.
Distributed under the terms of the MIT license.
Copyright 1999, Be Incorporated. All Rights Reserved.
This file may be used under the terms of the Be Sample Code License.
* Copyright 2005-2007, Haiku, Inc. All Rights Reserved.
* Distributed under the terms of the MIT license.
*
* Copyright 1999, Be Incorporated. All Rights Reserved.
* This file may be used under the terms of the Be Sample Code License.
*/
/** multiple-reader single-writer locking class */
/*! multiple-reader single-writer locking class */
// IMPORTANT:
// * nested read locks are not supported
@@ -19,15 +18,20 @@
#ifndef MULTI_LOCKER_H
#define MULTI_LOCKER_H
//#define TIMING 1
#include <OS.h>
const int32 LARGE_NUMBER = 100000;
#define MULTI_LOCKER_TIMING 0
#if DEBUG
# define MULTI_LOCKER_DEBUG DEBUG
#else
# define MULTI_LOCKER_DEBUG 0
#endif
class MultiLocker {
public:
MultiLocker(const char* semaphoreBaseName);
MultiLocker(const char* baseName);
virtual ~MultiLocker();
status_t InitCheck();
@@ -41,18 +45,25 @@ class MultiLocker {
bool WriteUnlock();
// does the current thread hold a write lock ?
bool IsWriteLocked(uint32 *stack_base = NULL,
bool IsWriteLocked(uint32 *stackBase = NULL,
thread_id *thread = NULL);
#if MULTI_LOCKER_DEBUG
// in DEBUG mode returns whether the lock is held
// in non-debug mode returns true
bool IsReadLocked();
#endif
private:
#if MULTI_LOCKER_DEBUG
// functions for managing the DEBUG reader array
void register_thread();
void unregister_thread();
void _RegisterThread();
void _UnregisterThread();
status_t fInit;
sem_id fLock;
int32* fDebugArray;
int32 fMaxThreads;
#else
// readers adjust count and block on fReadSem when a writer
// hold the lock
int32 fReadCount;
@@ -64,15 +75,14 @@ class MultiLocker {
// writers must acquire fWriterLock when acquiring a write lock
int32 fLockCount;
sem_id fWriterLock;
int32 fWriterNest;
#endif // MULTI_LOCKER_DEBUG
status_t fInit;
int32 fWriterNest;
thread_id fWriterThread;
uint32 fWriterStackBase;
int32 * fDebugArray;
int32 fMaxThreads;
#if TIMING
#if MULTI_LOCKER_TIMING
uint32 rl_count;
bigtime_t rl_time;
uint32 ru_count;
@@ -83,29 +93,30 @@ class MultiLocker {
bigtime_t wu_time;
uint32 islock_count;
bigtime_t islock_time;
uint32 reg_count;
bigtime_t reg_time;
uint32 unreg_count;
bigtime_t unreg_time;
#endif
};
class AutoWriteLocker {
public:
AutoWriteLocker(MultiLocker* lock)
: fLock(*lock)
:
fLock(*lock)
{
fLock.WriteLock();
}
AutoWriteLocker(MultiLocker& lock)
: fLock(lock)
:
fLock(lock)
{
fLock.WriteLock();
}
~AutoWriteLocker()
{
fLock.WriteUnlock();
}
private:
MultiLocker& fLock;
};
@@ -113,31 +124,36 @@ class AutoWriteLocker {
class AutoReadLocker {
public:
AutoReadLocker(MultiLocker* lock)
: fLock(*lock)
:
fLock(*lock)
{
fLocked = fLock.ReadLock();
}
AutoReadLocker(MultiLocker& lock)
: fLock(lock)
:
fLock(lock)
{
fLocked = fLock.ReadLock();
}
~AutoReadLocker()
{
Unlock();
}
void Unlock()
void
Unlock()
{
if (fLocked) {
fLock.ReadUnlock();
fLocked = false;
}
}
private:
MultiLocker& fLock;
bool fLocked;
};
#endif
#endif // MULTI_LOCKER_H
src/servers/app/drawing/HWInterface.h
+2
View File
@@ -50,7 +50,9 @@ class HWInterface : protected MultiLocker {
// locking
bool LockParallelAccess() { return ReadLock(); }
#if DEBUG
bool IsParallelAccessLocked() { return IsReadLocked(); }
#endif
void UnlockParallelAccess() { ReadUnlock(); }
bool LockExclusiveAccess() { return WriteLock(); }