[Sorry for comp.lang.c some folks may find it OT]
In Mulithreaded program, using Posix api, we do
pthread_mutex_lock(&Lock)
do some ops
if/else/if/else...complex stuff
pthread_mutex_unlock.
now assume because of complex if/else/if/else, thread got exited
without unlocking the mutex. :)
now the funny part is, other thread trying to acquire lock will
block. How to detect which thread [may be exited thread !!!!] has done
some wrong stuff. But how to track them?
Before few days i posted, why not to unlock all mutex acquired by
thread "A" when thread "A" is exiting/finishing/cancelled/thrown outof
system. !
Please note: from my code i can do manually logs and debug message
monitoring and can findout. What i am more interested in is, "How to
Autodetect and how to avoid, even code is done by ***Lazy***
programmer."
Any thought ?
Cheers,
Raxit
> Before few days i posted, why not to unlock all mutex acquired by
> thread "A" when thread "A" is exiting/finishing/cancelled/thrown outof
> system. !
The short answer is that this is just not how pthread's default mutex
implementation works. You could design a mutex system that worked this
way, and it would have some advantages and some disadvantages over the
way pthread's default mutex works. I think the disadvantages outweigh
the advantages.
The biggest problem is this: with the mutexes pthreads has given us,
it's easy to code mutexes as you suggest. With mutexes as you suggest,
there is no way to code the mutexes POSIX gives us.
The biggest disadvantage is performance. A scheme like you suggest
would require that every time a mutex is locked, some kind of
structure be created somewhere that can be located based on which
thread locked the mutex. Every time the mutex was unlocked, that
structure would have to be removed. There are some tricky cases, like
mutexes may not be unlocked in the same order they are locked.
The other disadvantage is the risk that incorrect behavior will cause
incorrect operation rather than failure. With the current API, if a
thread forgets to unlock a mutex and leaves the data that mutex
protects in an inconsistent state, no other thread will ever be
allowed to lock that mutex, which ensures it will not see the data in
the inconsistent change. With your modification, if a thread fails to
return data to consistent state and terminates while holding a mutex,
no other thread will be permitted to operate on the inconsistent data.
(Of course, the threads may pile up waiting for the mutex, so it won't
magically work perfectly. But failing completely is better than
operating incorrectly.)
DS
I will ignore the question of who/where has locked which mutex, since
Mr. Schwartz has already posted a possible solution.
Regarding the question of how to automatically unlock the mutex, the
approach we use at my company is RAII using C++. That is, we have two
classes, a mutex class and a lock class. To lock a mutex, we then
allocate a lock object on the stack, passing the mutex in as an
argument to the constructor. This then locks the mutex, and unlocking
is handled by the destructor which will be called at the end of the
method.
An example of the basic idea is the class below.
class Lock
{
public:
Lock(pthread_mutex_t* mutex) { theMutex = mutex;
pthread_mutex_lock(mutex); }
~Lock() { pthread_mutex_unlock(theMutex); }
private:
pthread_mutex_t* theMutex;
};
You code would then become:
Lock aLock(&mutex)
do some ops
if/else/if/else...complex stuff
// nothing (will unlock when aLock goes out of scope)
Of course, if you are using straight C then the above will not help
you at all.
Another idiom that I have heard people use is to prevent a deadlock
due to the locking order of mutexes. For example, if funcA() locks
mutexA,mutexB and funcB() locks mutexB,mutexA then you have a possible
deadlock. The idiom used is to always lock mutexes in the order of
their pointer address. So if &mutexA < &mutexB then lock
mutexA,mutexB. This only works when you know which mutexes you need to
lock at the beginning of a code section however.
HTH
Mark
> You code would then become:
> Lock aLock(&mutex)
> do some ops
> if/else/if/else...complex stuff
> // nothing (will unlock when aLock goes out of scope)
I don't recommend this approach. The problem is when you look at code
like:
// lots of code
Lock aLock(&ymutex);
// lots more code
}
}
}
}
func1();
}
Quick, is 'func1' called with the 'ymutex' locked or not? Which '}'
unlocks the mutex? Looking at the bottom of this code, you can't
tell.
If the end looked like:
// lots of code
}
func1();
ymutex.Unlock()
}
}
}
It's now obvious that 'func1' is called with the 'ymutex' held. A
person who adds code to the end of this function must make a conscious
choice to place it before or after the unlock, calling his new code
with the 'ymutex' held or not held as appropriate.
A possible compromise: Make your lockholders assert if destroyed with
the lock held in debug code, but make them release the lock silently
in release code. This way, you won't be screwed if some complex path
fails to release a lock in your release code, but you preserve the
ability to see exactly which mutexes are held and where they're
released, making your code more self-documenting.
DS
> > > You code would then become:
> > > Lock aLock(&mutex)
> > > do some ops
> > > if/else/if/else...complex stuff
> > > // nothing (will unlock when aLock goes out of scope)
> I don't recommend this approach. The problem is when you look at code
> like:
> // lots of code
> Lock aLock(&ymutex);
> // lots more code
> }
> }
> }
> }
> func1();
> }
> Quick, is 'func1' called with the 'ymutex' locked or not? Which '}'
> unlocks the mutex? Looking at the bottom of this code, you can't
> tell.
[...]
Great point! Humm, how would you feel if you had to work with code like:
// lots of code
Lock aLock(&ymutex);
// lots more code
}
}
}
// aLock.Unlock();
}
func1();
}
If you carefully place comments at the end-of-scope, then RAII and locking
might be more "reader friendly"...
{
Lock aLockOuter(omutex);
{
{
Lock aLockInner(imutex);
{
}
// aLockInner.Unlock();
}
}
// aLockOuter.Unlock();
}
Do you think that is feasible at all David?
Using RAII pattern is the only way that makes sense in C++. Otherwise
a function that exits via a thrown exception - will not be able to
unlock any mutexes that it may have locked (or otherwise be able to
release any resources that the funciton had acquired).
> The problem is when you look at code
> like:
>
> // lots of code
> Lock aLock(&ymutex);
> // lots more code
> }
> }
> }
> }
> func1();
> }
>
> Quick, is 'func1' called with the 'ymutex' locked or not? Which '}'
> unlocks the mutex? Looking at the bottom of this code, you can't
> tell.
The example above demonstrates a poorly-scoped function - and not any
drawbacks with the RAII pattern itself. If this function were
reasonably scoped, then the "aLock" would be held either for the
entire duration of the function or be held only briefly for, say, a
single function call.
> If the end looked like:
>
> // lots of code
> }
> func1();
> ymutex.Unlock()
> }
> }
>
> }
>
> It's now obvious that 'func1' is called with the 'ymutex' held.
How is it obvious that funct1() is called with the lock held? There is
no "lock()" call anywhere in evidence - so, for all we know func1()
itself locked the mutex. Assuming, of course, that the mutex was ever
locked at all.
The moral here of course is that anyone who edits a function by
looking only at its last few lines - is not someone you want making
changes to your software in the first place. A programmer has to
understand the entire function - before making any changes to it.
> A person who adds code to the end of this function must make a conscious
> choice to place it before or after the unlock, calling his new code
> with the 'ymutex' held or not held as appropriate.
So is it appropriate to add the code before or after the call to
unlock()? One of the problems with sprinkling explicit unlock() calls
throughout a program (aside being both error-prone and complicated) is
knowing where these unlock() calls are supposed to go in relation to
other operations. With scoped locking, the programmer does not have to
decide whether the code should be added before or after the unlock();
because the mutex is held for the entire duration of the function -
any code that would need the mutex to be unlocked - would belong in a
different function.
In other words, the locks in a properly-scoped program are self-
documenting; whereas the locks in a program that locks and unlocks its
mutexes at arbitrary points throughout its execution - is anything but
self-documenting.
Greg
You could add a try/catch block after the lock, but that takes you back
to having a single point of return, so you may as well just lock on
entry and unlock on exit.
--
Ian Collins.
Ra...@MyKavita.com schrieb:
> now assume because of complex if/else/if/else, thread got exited
> without unlocking the mutex. :)
> now the funny part is, other thread trying to acquire lock will
> block. How to detect which thread [may be exited thread !!!!] has done
> some wrong stuff. But how to track them?
Encapsulate the lock and unlock calls in debug builds by a funtion that
does some logging to a file. Always log a timestamp, the called function
(lock/unlock), the address of the Lock and the thread ID. Once you have
a deadlock simply analyze the log and search for the last occourencies
of the handle address of the last call to lock that never returned. You
will find the thread ID of the thread that never unlocked the mutex this
way.
If you frequently start threads, you should also track the thread starts
with some meaningful text to associate a meaningful context with the
thread ID.
Most Unix kernels provide atomic append functions for small amount of
output. So you do not need another mutex to serialize the output.
> Before few days i posted, why not to unlock all mutex acquired by
> thread "A" when thread "A" is exiting/finishing/cancelled/thrown outof
> system. !
You should not compensate for program bugs this way. It is not that
unlikely that other cleanup is also wrong in this case.
> Please note: from my code i can do manually logs and debug message
> monitoring and can findout. What i am more interested in is, "How to
> Autodetect and how to avoid, even code is done by ***Lazy***
> programmer."
There is no general way to autodetect deadlocks or unpaired mutex calls.
Only in some rare cases you may catch them safely.
But if C++ is a choice you may use a helper class whose constructor
aquires the lock and whose destructor releases the lock. This is solid
as rock! It is nearly impossible to prevent the destructor call even in
case of exceptions.
Marcel
> If the end looked like:
>
> // lots of code
> }
> func1();
> ymutex.Unlock()
> }
> }
> }
Yeah. Good workaround if you write extremely complex functions and
don't want to simplify them :)
Dmitriy V'jukov
> Greg Herlihy wrote:
>> On May 21, 12:08 pm, David Schwartz <dav...@webmaster.com> wrote:
>>> On May 21, 11:52 am, "Mark Holland" <kenshin...@htomail.com> wrote:
>>>
>>>> You code would then become:
>>>> Lock aLock(&mutex)
>>>> do some ops
>>>> if/else/if/else...complex stuff
>>>> // nothing (will unlock when aLock goes out of scope)
>>> I don't recommend this approach.
>>
>> Using RAII pattern is the only way that makes sense in C++. Otherwise
>> a function that exits via a thrown exception - will not be able to
>> unlock any mutexes that it may have locked (or otherwise be able to
>> release any resources that the funciton had acquired).
>>
> It can also lead to inconsistent data if an exception is thrown part way
> through a transaction.
In which case you're not paying attention to exception
safety. Exceptions shouldn't lead to inconsistent data, whether or not
the data is protected with a mutex.
> You could add a try/catch block after the lock, but that takes you back
> to having a single point of return, so you may as well just lock on
> entry and unlock on exit.
try/catch blocks are a tool of last resort for exception safety. The
RAII pattern is almost always preferable.
Anthony
--
Anthony Williams | Just Software Solutions Ltd
Custom Software Development | http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL
Apparently everything related to C is off topic in this newsgroup.
Only the very act of being nit-picky about the current ISO standard is
sanctioned here.
> In Mulithreaded program, using Posix api, we do
>
> pthread_mutex_lock(&Lock)
> do some ops
> if/else/if/else...complex stuff
> pthread_mutex_unlock.
>
> now assume because of complex if/else/if/else, thread got exited
> without unlocking the mutex. :)
> now the funny part is, other thread trying to acquire lock will
> block. How to detect which thread [may be exited thread !!!!] has done
> some wrong stuff. But how to track them?
This is a debugging problem. A Mutex is a lock that is assigned to a
particular thread. A good multi-threaded system should have a way, in
your debugger to know who owns the lock; in fact you might be able to
inspect that from your API (by having a time-out on your MUTEX) and
throw it into some log file somewhere -- i.e., directly instrument
your code to do this sort of analysis. Whenever I have implemented my
own Mutexes from scratch, what I did was I would shove the __FILE__,
__LINE__ macros into the mutex structure to identify the owner because
the point in the code where the Mutex got owned without cleared was
usually more relevant than the thread ID.
> Before few days i posted, why not to unlock all mutex acquired by
> thread "A" when thread "A" is exiting/finishing/cancelled/thrown outof
> system. !
As I said -- this is a debugging problem. You have an error in your
code. You need to isolate the error, not try to vacate the problem
with some trick.
> Please note: from my code i can do manually logs and debug message
> monitoring and can findout. What i am more interested in is, "How to
> Autodetect and how to avoid, even code is done by ***Lazy***
> programmer."
This is like the halting problem -- there isn't really a full-proof
way. You can, of course, make a macro with both acquires the lock and
releases it:
#define WITH_MUTEX(mutex,code) { \
int h = mutexAcquire (mutex); \
if (SUCCESS == h) { \
do { code } while (0); \
mutexRelease (mutex); \
} \
}
And in fact you could put in a try+catch thing for C++ to make sure
that you don't escape that way. So you can then set a coding
standard, and in fact have some tool grep through the code and
complain about bare calls to the mutex primitives instead of using the
macro above. But of course, you cannot avoid longjmp() or coroutine
yields anyways, so even with this limited usage there are no
guarantees.
--
Paul Hsieh
http://www.pobox.com/~qed/
http://bstring.sf.net/
>Apparently everything related to C is off topic in this newsgroup.
>Only the very act of being nit-picky about the current ISO standard is
>sanctioned here.
So, Paul, what do you think about the tradeoffs of using 10W20
as compared to 10W30? And does it make a difference whether it is
synthetic or natural? My wife has cut down on her car use, but
it isn't uncommon for her to pick me up in her 95 Accord after work,
especially if we have to go grocery shopping. The temperature range
here is from about -45C to +35C, with -15C or so being pretty common
in winter outside of the cold snaps; the summer tends to about +25C.
This is, of course, a real-world C situation (since I am a C
programmer), so I figure comp.lang.c oughta be cool with it.
--
"And that's the way it is." -- Walter Cronkite
Some people think that posting bullshit claiming that
things-related-to-C are unwelcome in the newsgroup [apparently
comp.lang.c, only one of the three Paul Hsieh crossposted to] is just
what the doctor ordered. It's certainly a lie about comp.lang.c, and
has zero relevance to comp.unix.programmer or comp.programming.threads.
It is worth noting, that almost nothing can possibly be topical in all
three of the newsgroups to which this thread is crossposted.
I seem to have removed my anti-gmail filter to soon. I'll just
reintroduce one for Paul "Nitwit" Hsieh.
--
Ian Collins.
The problem with Hsieh is that he is often violently off-topic here
(in c.l.c), but occasionally makes useful and interesting topical
quotes.
I have set follow-ups to c.l.c, where this is entirely off-topic.
The other groups may well be topical.
--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.
** Posted from http://www.teranews.com **
> If you carefully place comments at the end-of-scope, then RAII and locking
> might be more "reader friendly"...
>
> {
> Lock aLockOuter(omutex);
> {
> {
> Lock aLockInner(imutex);
> {
> }
> // aLockInner.Unlock();
> }
> }
> // aLockOuter.Unlock();
>
> }
>
> Do you think that is feasible at all David?
Sure, but then why comment them out? At a minimum, have that do
something in debug mode to detect missing 'Unlock' comments.
DS
Are you sure that's what you meant to do, or did you want to redirect
followups to the *other* groups? (This is cross-posted to all three
groups; please limit followups as appropriate.)
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Okay. I agree with you on the basis that I am biased; I am a C programmer at
heart. In current C++ I have to doctor the POSIX callbacks with extern "C"
and when I am done it sure looks like more work that any C version.
Anyway...
wrt current C++ Standard, the "special feature" of RAII in general does tend
to come in handy wrt automating the "opening and closing" function/procedure
calls into an underlying synchronization primitive API. The basic technique
simplifies creating exception friendly code paths indeed. One question, how
do you "efficiently" handle RAII end-of-scope actions when an exception
arises in a purely thread based program? The answer is that you never allow
an exception to escape a particular region with inconsistent data. The means
that you need to take great care inside critical regions. Why would a thread
throw inside a critical region without ensuring coherent data __before__ it
actually executed the `throw ...' function. Since a C++ exception needs to
be explicitly called, it follows that the thread which is raise said
exception would assert the shared state accordingly _before_ it actually
throws anything.
With proper documentation and strict __PRE-THROW_CONDITIONS_, IMVHO,
RAII-based synchronization schemes can provide a great alternative to
manually placing explicit calls to `mutex:lock/unlock' functions all over
the place. I mean, if I am forced to program in C++, well, the transitive
property dictates that I should take advantage of RAII in the presence of a
exceptions, and the simplicity it casts upon the locking scheme. In C, lock
calls and return checking are usually explicit unless some black-magic macro
scheme is setup...
One more point, IMVHO, a purely thread-based program should not need to
depend on so-called robust mutex semantics. In other words, I can understand
that processes can get terminated while they happen to be holding a mutex
protecting an important data-structure, however, the individual threads in
said processes should not ever be able to terminate unexpectedly. There are
some exceptions, for instance, if your running a plug-in system using a
purely threaded environment then you have to treat the nature of the
plug-ins like processes than can do unexpected things, such as terminate
prematurely...
Shared library code which maintains some "persistent state" somewhere _and_
does indeed invoke callbacks into "user-code" which can possibly mutate said
state _does_ need to be very weary of so-called "Intentionally BAD, or
buggy, user-code"... Humm...
You can expand upon the the RAII technique here to ensure this does not
happen by adding an explicit unlock to the object, something like
struct Locker
{
mutext_t& mutex;
bool isLocked;
Locker( mutext_t& m ) : mutex(m) {
m.lock(); isLocked = true; }
void unlock() { m.unlock(); isLocked = false; }
~Locker() { assert( !isLocked ); }
};
--
Ian Collins.
> You can expand upon the the RAII technique here to ensure this does not
> happen by adding an explicit unlock to the object, something like
>
> struct Locker
> {
> mutext_t& mutex;
> bool isLocked;
>
> Locker( mutext_t& m ) : mutex(m) {
> m.lock(); isLocked = true; }
>
> void unlock() { m.unlock(); isLocked = false; }
>
> ~Locker() { assert( !isLocked ); }
>
> };
Almost. The problem is that this won't work well in release mode. What
you want is:
~Locker()
{
if(isLocked)
{
assert(false);
m.Unlock();
}
}
This way, in debug mode you can catch missing unlock statement. In
release mode, a missing unlock won't hurt you (very much).
At least, I would suggest this in a case where a missing unlock is the
most likely reason the mutex wouldn't be unlocked and where it's
preferable to continue possible incorrect operation rather than
terminate as soon as possible.
DS
But yes, that is safer, the code was only intended to illustrate the
technique.
--
Ian Collins.
A transaction by definition is atomic. So either the transaction will
complete or its effects are "rolled back" as if the transaction had
never started. In this case, the destructor of the Transaction RAII
object can tell whether the transaction completed or not - and if it
did not complete, the Transaction object then rolls back any changes.
So there is no chance that a thrown exception would lead to
inconsistent data for any C++ program that uses RAII objects to
implement transactions.
Moreover, any code that leaves data in an inconsistent state when an
exception is thrown is - by definition - not exception-safe. And the
easiest way to make C++ code exception-safe, is to adopt the RAII
pattern. A RAII object is able to guarantee that a function properly
cleans up after itself in the event of a thrown exception.
> You could add a try/catch block after the lock, but that takes you back
> to having a single point of return, so you may as well just lock on
> entry and unlock on exit.
..which is exactly the behavior that RAII guarantees - and guarantees
more reliably than any other technique.
Greg
--
Ian Collins.
Yup. Because, having snipped it, it only deals with topicality on
c.l.c. Confusing, isn't it? I think this is the right thing to
do.