Burton M. Strauss III wrote:
I understand WHAT you're trying to do, but I have some questions....
0) Single threading ntop on the isLocked setting should prevent us from
stepping on it, but do we want to expand the protected area to be all of the
lock values? It's theoretically possible to get the other value stepped on,
although I'll grant it's probably impossible in real life...
I can't follow you here. Can you elaborate please?
Mutexes are used because two or more concurrent threads try to access the same resource. This means that those threads can all access at the same time the same resource. If two threads are calling concurrently _accessMutex() they will both change mutexId->lockAttempt*, the last thread will overwrite the values, but only the first thread will actually get the mutex as the second one (the one that overwrote the value) will be stuck at pthread_mutex_lock. Therefore:
1) it looks like you've butchered the lockAttempt stuff...
You moved the setting of the field AFTER the lock of the mutex. The point
of the change was to load the field BEFORE, so that if the
pthread_mutex_lock() blocked, the lockAttempt (blocked) stuff was set and
could be retrieved by other threads (e.g. the web server for info.html).
The code in _accessMutex() should look like this:
strcpy(mutexId->lockAttemptFile, fileName);
mutexId->lockAttemptLine=fileLine;
rc = pthread_mutex_lock(&(mutexId->mutex));
mutexId->lockAttemptFile[0] = '\0';
mutexId->lockAttemptLine=0;
2) Why did you move the self locked mutex test?
According to the POSIX standard, it is not required to report errors such as
attempting to lock a mutex you've already locked. In fact, just about ANY
behavior is possible - so I wanted the log message to occur BEFORE the code
(potentially) deadlocked.
http://www.opengroup.org/onlinepubs/7908799/xsh/threads.html and
specifically
http://www.opengroup.org/onlinepubs/7908799/xsh/pthread_mutex_lock.html
"If the mutex type is PTHREAD_MUTEX_NORMAL, deadlock detection is not
provided. Attempting to relock the mutex causes deadlock. If a thread
attempts to unlock a mutex that it has not locked or a mutex which is
unlocked, undefined behavior results.
If the mutex type is PTHREAD_MUTEX_ERRORCHECK, then error checking is
provided. If a thread attempts to relock a mutex that it has already locked,
an error will be returned. If a thread attempts to unlock a mutex that it
has not locked or a mutex which is unlocked, an error will be returned.
If the mutex type is PTHREAD_MUTEX_RECURSIVE, then the mutex maintains the
concept of a lock count. When a thread successfully acquires a mutex for the
first time, the lock count is set to one. Every time a thread relocks this
mutex, the lock count is incremented by one. Each time the thread unlocks
the mutex, the lock count is decremented by one. When the lock count reaches
zero, the mutex becomes available for other threads to acquire. If a thread
attempts to unlock a mutex that it has not locked or a mutex which is
unlocked, an error will be returned.
If the mutex type is PTHREAD_MUTEX_DEFAULT, attempting to recursively lock
the mutex results in undefined behavior. Attempting to unlock the mutex if
it was not locked by the calling thread results in undefined behavior.
Attempting to unlock the mutex if it is not locked results in undefined
behavior."
Since we init w/ NULL (PTHREAD_MUTEX_DEFAULT) behavior COULD be
PTHREAD_MUTEX_NORMAL which would deadlock. I don't think this would cause a
problem either way with any of our normal environments, but with two new
candidate gcc thread libraries on the horizon, plus this being an undefined
behavior situation, I wanted to make ntop's stuff clear.
1. modify values (e.g. mutexId->lockAttempt*) only when it's time to do it (-> when you've got the lock on the mutex)
2. do not trust values such as mutexId->isLocked unless you're sure that the value does not change. For instance if you write:
if(mutexId->isLocked) {
print "Error!"
}
lockMutex(...)
it's a wrong statement *unless* you have a super-mutex that makes sure that the isLocked values does not change
Bottom line: you've done a great work with mutexes. I have just fixed some issues.
Regards, Luca
I have a patch I was working on to add the PID# to the reports and lock
data. I'll include the fixes for #1 and #2 above in that fix (#181). #0
I'll leave until I hear from you...
-----Burton
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf
Of [EMAIL PROTECTED]
Sent: Thursday, December 19, 2002 4:15 AM
To: [EMAIL PROTECTED]
Subject: [Ntop-dev] New ntop commit (author deri)
Update of /export/home/ntop/ntop
In directory jabber:/tmp/cvs-serv5092
Modified Files:
hash.c ntop.c sessions.c util.c webInterface.c
Log Message:
Added fixes for mutex warnings. A new "super mutex" named stateChangeMutex
has been added in order to serialize activities on mutexes.
_______________________________________________
Ntop-dev mailing list
[EMAIL PROTECTED]
http://listgateway.unipi.it/mailman/listinfo/ntop-dev
_______________________________________________
Ntop-dev mailing list
[EMAIL PROTECTED]
http://listgateway.unipi.it/mailman/listinfo/ntop-dev
-- Luca Deri <[EMAIL PROTECTED]> http://luca.ntop.org/ Hacker: someone who loves to program and enjoys being clever about it - Richard Stallman _______________________________________________ Ntop-dev mailing list [EMAIL PROTECTED] http://listgateway.unipi.it/mailman/listinfo/ntop-dev
