pthread_mutex_delete on almost-used mutex?
Ivan Skytte Jørgensen
I have a question regarding destroying a mutex before it is completely
unused.
Consider:
Thread A:
a1 pthread_mutex_lock(mtx);
a2 pthread_cond_signal(cond);
a3 pthread_mutex_unlock(mtx);
<never references the mutex again>
Thread B:
b1 pthread_mutex_lock(mtx);
b2 pthread_cond_wait(cond,mtx);
b3 pthread_mutex_unlock(mtx);
b4 pthread_destroy(mtx)
The interesting thing is that when (a3) is done it transfers ownership
of the mutex to thread B. Thread B wakes up, unlocks the mutex, and
destroys it. And this can happen before the call (a3) has returned.
Is the program above well-defined ?
After pthread_mutex_unlock() in step a3 has internally transferred
ownership to thread B there should be no reason for it to reference
the mutex again, but what does the Posix standard say about this?
The use of the condition object is not essential, it could be any
other mechanism for signalling between the two threads. The problem
arose when Bryan Ischo gave me an MTA trace file that had this
scenario in it and the analyzer program did not like it.
Regards,
Ivan
Alexander Terekhov
Ivan Skytte Jørgensen wrote:
[...]
> The interesting thing is that when (a3) is done it transfers ownership
> of the mutex to thread B. Thread B wakes up, unlocks the mutex, and
> destroys it. And this can happen before the call (a3) has returned.
>
> Is the program above well-defined ?
Sure.
>
> After pthread_mutex_unlock() in step a3 has internally transferred
> ownership to thread B there should be no reason for it to reference
> the mutex again, but what does the Posix standard say about this?
See "Destroying Mutexes" at
http://www.opengroup.org/onlinepubs/007904975/functions/pthread_mutex_destroy.html
<quote>
Implementations are required to allow an object to be destroyed and
freed and potentially unmapped (for example, lines A and B) immediately
after the object is unlocked (line C).
</quote>
regards,
alexander.
David Schwartz
"Ivan Skytte Jørgensen" <isj@dont_forget_to_remove_this.image.dk.invalid>
wrote in message news:40859B9C...@image.dk...
> I have a question regarding destroying a mutex before it is completely
> unused.
I assume we're talking about the pthreads standard here, not what any
implementation happens to do.
> Consider:
> Thread A:
> a1 pthread_mutex_lock(mtx);
> a2 pthread_cond_signal(cond);
> a3 pthread_mutex_unlock(mtx);
> <never references the mutex again>
> Thread B:
> b1 pthread_mutex_lock(mtx);
> b2 pthread_cond_wait(cond,mtx);
> b3 pthread_mutex_unlock(mtx);
> b4 pthread_destroy(mtx)
>
> The interesting thing is that when (a3) is done it transfers ownership
> of the mutex to thread B.
No, it simply unlocks the mutex.
> Thread B wakes up, unlocks the mutex, and
> destroys it. And this can happen before the call (a3) has returned.
Before it has returned, but not before it has unlocked the mutex.
> Is the program above well-defined ?
Yes, a mutex may be destroyed so long as it's not locked.
> After pthread_mutex_unlock() in step a3 has internally transferred
> ownership to thread B there should be no reason for it to reference
> the mutex again, but what does the Posix standard say about this?
"It shall be safe to destroy an initialized mutex that is unlocked.
Attempting to destroy a locked mutex results in undefined behavior."
> The use of the condition object is not essential, it could be any
> other mechanism for signalling between the two threads. The problem
> arose when Bryan Ischo gave me an MTA trace file that had this
> scenario in it and the analyzer program did not like it.
The analyzer is broken in either not correctly ordering its log events
or not correctly taking into account the lack of ordering in its log events.
If you do this:
log("unlocking mutex")
unlock();
log("unlocked mutex")
Then the mutex may be locked or unlocked in-between the two calls.
There's no way to know which.
DS
Jan-Henrik Haukeland
> "Ivan Skytte Jørgensen" <isj@dont_forget_to_remove_this.image.dk.invalid>
> wrote in message news:40859B9C...@image.dk...
>
>> I have a question regarding destroying a mutex before it is completely
>> unused.
>
> I assume we're talking about the pthreads standard here, not what any
> implementation happens to do.
>
>> Consider:
>> Thread A:
>> a1 pthread_mutex_lock(mtx);
>> a2 pthread_cond_signal(cond);
>> a3 pthread_mutex_unlock(mtx);
>> <never references the mutex again>
>> Thread B:
>> b1 pthread_mutex_lock(mtx);
>> b2 pthread_cond_wait(cond,mtx);
>> b3 pthread_mutex_unlock(mtx);
>> b4 pthread_destroy(mtx)
>>
>> Is the program above well-defined ?
>
> Yes, a mutex may be destroyed so long as it's not locked.
What if a1 execute before b4? Just my $0.02
--
Jan-Henrik Haukeland
David Butenhof
Ah, but a very worthwhile $0.02. (If I can presume that you meant
"immediately before" -- as in, a1 might execute after b3 and before b4. So
to speak.)
Yes, the original pseudocode is clearly intended to "force" the desired and
presumed order of events in that Thread B is expected to wait for Thread A
to signal the condition variable, ensuring that Thread A will be at point
a3 or beyond when Thread B reaches b4.
In fact, however, this is an incorrect presumption because the code is badly
broken. Without a predicate on the condition wait, there is no guarantee
that Thread B cannot complete before Thread A runs at all.
So, no; the "program" is not at all well defined -- in fact, it's completely
illegal.
However, we can also allow that the construction of the program was merely a
philosophical exercise to set up an assertion that one thread is deleting a
mutex recently unlocked by itself and another thread, such that the other
thread might still be within the dynamic scope of pthread_mutex_unlock.
And in that sense; yes, as long as the mutex is unlocked and there are no
waiters (you can ensure no other thread might be trying/planning to lock;
whether they might be queued, spinning, or on some application code path
inevitably leading towards a use of the mutex), you can safely delete it.
But also beware that this is a tricky area of implementation, and you are
not entirely unlikely to run into some implementation that's done it wrong
by referencing the mutex (e.g., to record performance or debugging
information) "on the way out" of the lock operation. While you'd end up
with a valid bug report and perhaps a sense of personal justification,
you'd still have a program that wouldn't work on that platform. Sometimes,
you can't reasonably avoid dynamic and disposable locks; however as a
matter of porting pragmatics you're better off sticking to static locks if
you can, or to cache and save your "preowned" locks rather than destroying
them. (And if you eventually flush the cache of ancient petrified locks,
you ought to be reasonably safe even on one of those implementations that
mucked it up.)
--
/--------------------[ David.B...@hp.com ]--------------------\
| Hewlett-Packard Company Tru64 UNIX & VMS Thread Architect |
| My book: http://www.awl.com/cseng/titles/0-201-63392-2/ |
\----[ http://homepage.mac.com/dbutenhof/Threads/Threads.html ]---/
David Schwartz
"Jan-Henrik Haukeland" <hauk...@tildeslash.com> wrote in message
news:m3k7081...@7tvrdc1.cm.chello.no...
>>> Consider:
>>> Thread A:
>>> a1 pthread_mutex_lock(mtx);
>>> a2 pthread_cond_signal(cond);
>>> a3 pthread_mutex_unlock(mtx);
>>> <never references the mutex again>
>>> Thread B:
>>> b1 pthread_mutex_lock(mtx);
>>> b2 pthread_cond_wait(cond,mtx);
>>> b3 pthread_mutex_unlock(mtx);
>>> b4 pthread_destroy(mtx)
>>>
>>> Is the program above well-defined ?
>>
>> Yes, a mutex may be destroyed so long as it's not locked.
>
> What if a1 execute before b4? Just my $0.02
In real-world code, this is impossible. In order for thread A to attempt
to lock the object, the object must be in a "locatable" state. Step 'b2' is
supposed to represent making the object unlocatable.
DS
Jan-Henrik Haukeland
In the real world you should not be supprised if b3->a1->b4 occurred :)
--
Jan-Henrik Haukeland
David Butenhof
b2 doesn't make anything 'locatable' -- this code excerpt presumes that both
the mutex and condition variable are visible to both threads at entry; no
protocol exists for visibility to either otherwise.
I hope you're not saying that spurious wakes from the condition wait cannot
occur? They can; and if Thread B begins execution first, AND receives a
spurious wake, it can indeed run to completion before Thread A ever starts.
This is precisely the sort of real-world situation that many people
overlook during design.
David Schwartz
>>>>> Consider:
>>>>> Thread A:
>>>>> a1 pthread_mutex_lock(mtx);
>>>>> a2 pthread_cond_signal(cond);
>>>>> a3 pthread_mutex_unlock(mtx);
>>>>> <never references the mutex again>
>>>>> Thread B:
>>>>> b1 pthread_mutex_lock(mtx);
>>>>> b2 pthread_cond_wait(cond,mtx);
>>>>> b3 pthread_mutex_unlock(mtx);
>>>>> b4 pthread_destroy(mtx)
>>>>>
>>>>> Is the program above well-defined ?
>>>>
>>>> Yes, a mutex may be destroyed so long as it's not locked.
>>>
>>> What if a1 execute before b4? Just my $0.02
>>
>> In real-world code, this is impossible. In order for thread A to
>> attempt
>> to lock the object, the object must be in a "locatable" state. Step 'b2'
>> is
>> supposed to represent making the object unlocatable.
>
> In the real world you should not be supprised if b3->a1->b4 occurred :)
You should be very surprised. In order for thread A to ever call 'a1',
it has to *find* the mutex. And step 'b2' is only called *after* the mutex
is made unlocatable. That's what 'b2' is supposed to represent. This is
pseudo-code, not real code.
When you see step b2, think "whatever synchronization is needed to make
sure that no other thread can lock this mutex".
DS
SenderX
> waiters (you can ensure no other thread might be trying/planning to lock;
> whether they might be queued, spinning, or on some application code path
> inevitably leading towards a use of the mutex), you can safely delete it.
I usually follow a strict, simple rule that dictates the time its ok to
delete "global" mutex's:
"Since they are not refcounted, you must make sure that EVERY thread that
might use a global mutex must be JOINED "before" the mutex destruction can
occur." This simply ensures that there are no references to the object
during and after its deletion.
Or, I guess you could use atomic_ptr to protect the mutex...
:)
P.S.
Personally, If you think its ok to delete mutexs if they are unlocked...
"With no regard to the fact that other threads could just be entering the
mutex's lock function". Your just asking/begging for a crash?:
static Mutex g_pLock = new Mutex;
Thread A
---------------------
a1: g_Lock.Lock();
a2: sched_yield();
a3: g_Lock.Unlock();
// call mutex dtor
a4: delete g_pLock;
Threads B - X
-----------------
bx1: g_Lock.Lock()
{
// Inside lock function, using g_Lock's "this" pointer
bx2: this->m_LockState = something;
bx3: Some more stuff to acquire
}
bx4: sched_yield();
bx5: g_Lock.Unlock();
Lets say the execution sequence goes like this:
a1
a2
bx1
a3
a4
***bx2
bx3
bx4
bx5
*** You go boom!
;)
Bryan Ischo
At least for my code, from which this discussion was initiated, and
for which the pseudo-representative set of pthreads calls that Ivan
presented were made, your statements are all correct. The condition
which is being waited on in b2 is the "object containing mutex is no
longer referenced by anybody else" condition; it is tested in a loop
like this:
while (objectIsUsedByAnyone) {
pthread_cond_wait(objectCondition, objectMutex);
}
You are right in assuming that the whole point of b2 is to wait until
no one (except the last guy to use the object, who locked it,
decremented its reference count, saw that it was now 0, signalled its
condition, and unlocked it - this is represented by thread A in Ivan's
example) else can be referencing it.
It is definitely true that this *could* be gotten wrong in code - but
I don't think I got it wrong. I have a test case which runs through
literally millions of these sequences and it works just fine.
Furthermore, if I could show you the code (maybe someday, it's still
under development and I haven't decided whether or not to GPL it), I
think I could prove to you that the object containing the mutex in
question is no longer locatable by the time that thread B unlocks and
destroys its mutex.
Thanks, and best wishes,
Bryan
David Butenhof
> Personally, If you think its ok to delete mutexs if they are unlocked...
> "With no regard to the fact that other threads could just be entering the
> mutex's lock function". Your just asking/begging for a crash?:
Since this is a reply to my post, there's something in here that makes me
suspect that you think you're disagreeing with something I said. But if
that's the case, then you didn't read my post carefully enough.
But if you intended to agree with me and add your own comments, hey, that's
fine. ;-)
In general, you simply don't destroy resources that some other thread might
want. That's like pulling a chair out from under someone as they sit down.
Some might think that's a cute "practical joke", but any developer who
plays such tricks on their own application has, uh, "issues".
David Butenhof
> At least for my code, from which this discussion was initiated, and
> for which the pseudo-representative set of pthreads calls that Ivan
> presented were made, your statements are all correct. The condition
> which is being waited on in b2 is the "object containing mutex is no
> longer referenced by anybody else" condition; it is tested in a loop
> like this:
>
> while (objectIsUsedByAnyone) {
> pthread_cond_wait(objectCondition, objectMutex);
> }
>
> You are right in assuming that the whole point of b2 is to wait until
> no one (except the last guy to use the object, who locked it,
> decremented its reference count, saw that it was now 0, signalled its
> condition, and unlocked it - this is represented by thread A in Ivan's
> example) else can be referencing it.
>
> It is definitely true that this *could* be gotten wrong in code - but
> I don't think I got it wrong. I have a test case which runs through
> literally millions of these sequences and it works just fine.
> Furthermore, if I could show you the code (maybe someday, it's still
> under development and I haven't decided whether or not to GPL it), I
> think I could prove to you that the object containing the mutex in
> question is no longer locatable by the time that thread B unlocks and
> destroys its mutex.
The point is that the "illustrative" sequence of "pseudocode" was wrong. It
was, in fact, "anti illustrative". David Schwartz, for whatever reason, was
willing to grant that what Ivan wrote had no useful relationship to what he
meant, but I happen to think that was, overall, a disservice to the
discussion. Ivan presented a programmatically literal "pseudocode" sequence
(literal enough to include specific API calls, which was wildly
inappropriate if it was as abstract as you say), and asked directly "is
this a well formed program"? Well, no, it wasn't.
Note for future reference:
If you mean "at this point we ensure consistent state", don't write
"pthread_cond_wait" -- because that doesn't and cannot ensure any such
thing, and both strongly and directly implies a lack of understanding of
the basic nature of the problem which needs to be corrected before any more
substantial discussion can begin.
In "pseudocode" at this level of detail you can simply write "construct a
bridge", and we can at least initially presume it's reasonable to grant the
benefit of the doubt that you know how to construct a bridge. Write "set
out a nail", (which is essentially what Ivan did) and there are many
legitimate reasons to form the basic concern that perhaps you have no idea
at all how to even begin constructing a bridge.
Ultimately, if you're satisfied with the code and your testing of it, what's
the point of the question? And if you're not, how do you expect anyone to
rationally critique such a rough sketch of the algorithm as the
"pseudocode" that was submitted here? I fail to see what you (or Ivan)
expected.
Alexander Terekhov
David Butenhof wrote:
[...]
> Ivan presented a programmatically literal "pseudocode" sequence
> (literal enough to include specific API calls, which was wildly
> inappropriate if it was as abstract as you say), and asked directly "is
> this a well formed program"? Well, no, it wasn't.
Well, you can't say that. He simply cut off "b5".
#define b5 ;
;-)
regards,
alexander.
David Butenhof
Nevertheless, the pseudocode posted did not provide b5 and therefore cannot
be considered "well formed". (Besides, I expect several other additions
would be needed to meet even minimal C language specifications for a "well
formed" source file. For one thing, "<never references the mutex again>" is
not a well-formed C language comment token!
;-)
Bryan Ischo
> Ultimately, if you're satisfied with the code and your testing of it, what's
> the point of the question? And if you're not, how do you expect anyone to
> rationally critique such a rough sketch of the algorithm as the
> "pseudocode" that was submitted here? I fail to see what you (or Ivan)
> expected.
Ivan's MTA analyzer produced questionable output for my code. He
believed that my code was at fault, and I believed that the analyzer
was at fault. We agreed that he would ask the question on this forum
to get the opinions of experts on the subject who might be able to
help decide the issue for us. That was the point of the question.
I think that his expectation that the question would be rationally
critiqued was validated by the actual valid critiques that followed.
We got the answer that we were looking for; the manner in which the
question was posed was not perfect but it did the job. There were
definitely unwritten assumptions in the pseudocode that Ivan used to
illustrate the question, which has good points (readability,
conciseness), and bad points (implicit assumptions which must be
accepted by the reader before the question can properly be addressed).
David S. looked beyond the gaps and assumed that the question being
asked was the one that we were interested in, and answered it. Others
pointed out that the question was in fact incomplete. Both of these
are great because not only did we get an answer to our question, but
important aspects of the problem surrounding the actual question were
identified as well, which allowed me to verify the sanity of my
approach.
In terms of what I expected, I expected that the question that Ivan
and I had together, and that Ivan asked, would be answered in a way
that would make it clear that we understood the problem and that one
of us was correct. As for what I expected to come of my post -
nothing, I was just giving information in hopes that it would clarify
the question by indicating the source from which it question was
drawn.
Thanks,
Bryan
SenderX
> suspect that you think you're disagreeing with something I said. But if
> that's the case, then you didn't read my post carefully enough.
>
> But if you intended to agree with me and add your own comments, hey,
that's
> fine. ;-)
Yeah, I agree with you.
Another simple setup for persistent locks is to setup a big static array of
mutexs. Like:
static Mutex m_Locks[1024];
static volatile int iIndex = 0;
And just assign index numbers to objects:
int GetLock()
{
int i = iIndex++;
if ( i >= 1024 )
{
iIndex = i = 0;
}
return i;
}
Mutex& GetLockRef( int i )
{
return m_Locks[i];
}
// Any object
class CObj
{
public:
CObj() { m_iLock = GetLock(); }
void Lock()
{
GetLockRef(m_iLock).Lock();
}
void Unlock()
{
GetLockRef(m_iLock).Unlock();
}
int m_iLock;
};
> In general, you simply don't destroy resources that some other thread
might
> want. That's like pulling a chair out from under someone as they sit down.
> Some might think that's a cute "practical joke", but any developer who
> plays such tricks on their own application has, uh, "issues".
lol!
:)
Alexander Terekhov
SenderX wrote: ...
Next Monday is not your day, Sender.
regards,
alexander.
SenderX
>
> Next Monday is not your day, Sender.
I hate monday!
David Schwartz
> The point is that the "illustrative" sequence of "pseudocode" was wrong.
> It
> was, in fact, "anti illustrative". David Schwartz, for whatever reason,
> was
> willing to grant that what Ivan wrote had no useful relationship to what
> he
> meant, but I happen to think that was, overall, a disservice to the
> discussion. Ivan presented a programmatically literal "pseudocode"
> sequence
> (literal enough to include specific API calls, which was wildly
> inappropriate if it was as abstract as you say), and asked directly "is
> this a well formed program"? Well, no, it wasn't.
The code sequence was not illustrative of what I think his question was.
His question is, whether another thread seeing the mutex unlocked guarantees
sufficient completion of 'pthread_mutex_unlock', even if it hasn't returned,
to make 'pthread_mutex_delete' safe?
DS
Joe Seigh
Alexander Terekhov wrote:
>
> SenderX wrote: ...
>
> Next Monday is not your day, Sender.
>
So, whose day is it then? :)
Joe Seigh
David Butenhof
> Another simple setup for persistent locks is to setup a big static array
> of mutexs. Like:
>
> static Mutex m_Locks[1024];
> static volatile int iIndex = 0;
>
> And just assign index numbers to objects:
Absolutely. This is, essentially, a simplistic form of hashing to spread
contention across a (possibly large and potentially tunable) set of locks.
Probably a good idea. Especially if you can come up with a better hash
function than simply ++iIndex, to distribute the contention in some useful
way.
Alexander Terekhov
David Butenhof wrote:
>
> SenderX wrote:
>
> > Another simple setup for persistent locks is to setup a big static array
> > of mutexs. Like:
> >
> > static Mutex m_Locks[1024];
> > static volatile int iIndex = 0;
> >
> > And just assign index numbers to objects:
>
> Absolutely. This is, essentially, a simplistic form of hashing to spread
> contention across a (possibly large and potentially tunable) set of locks.
> Probably a good idea. Especially if you can come up with a better hash
> function than simply ++iIndex, to distribute the contention in some useful
> way.
Sender is on Windows. Windows mutexes are recursive. In POSIX
(unless you use PTHREAD_MUTEX_RECURSIVE locks) such distribution
is quite risky business (unless you can forbid nested locking).
Oder?
regards,
alexander.
David Schwartz
"Alexander Terekhov" <tere...@web.de> wrote in message
news:408CF5AF...@web.de...
> Sender is on Windows. Windows mutexes are recursive. In POSIX
> (unless you use PTHREAD_MUTEX_RECURSIVE locks) such distribution
> is quite risky business (unless you can forbid nested locking).
>
> Oder?
It's still risky because it becomes much harder to enforce lock
ordering.
DS
SenderX
Actually, I'm on both windows and some unix's now. I've been fine tuning my
library, and finishing up the test suites.
> In POSIX
> (unless you use PTHREAD_MUTEX_RECURSIVE locks) such distribution
> is quite risky business (unless you can forbid nested locking).
Yes Alex. There are issues with this setup...
For instance... A single thread can't lock two objects that received their
locks from the same array:
Thread A
-------------------
ObjA.Lock();
ObjB.Lock();
< critical section >
ObjB.Unlock();
ObjA.Unlock();
That will deadlock all over the place!
In order for this to be safe... ObjA and ObjB must have received their locks
from "different" arrays.
I should have mentioned this caveat!
:O
P.S.
The hash'd lock API in my library uses per-thread flags to inform the API's
lock function that the thread has locked an object from a certain array. If
the lock function detects a thread trying to lock two objects, it returns a
deadlock error.
Alexander Terekhov
SenderX wrote:
[...]
> ObjA.Lock();
> ObjB.Lock();
> < critical section >
> ObjB.Unlock();
> ObjA.Unlock();
>
> That will deadlock all over the place!
It won't deadlock if the hidden lock ("shared" under the cover by
ObjA and ObjB) is recursive.
[...]
> The hash'd lock API in my library uses per-thread flags...
Per-thread flags can be used to turn nonrecursive locks into
recursive ones.
given:
key - tsd flag key
lock - mutex (non-recursive lock)
count - unsigned
init {
tsd::init(&key);
lock::init(&lock);
count = 0;
}
destroy {
lock::destroy(&lock);
tsd::destroy(&key);
}
lock {
if (!tsd::get(&key)) {
tsd::set(&key, true);
lock.acquire(); // assume throw()-nothing
count = 1;
}
else {
++count;
}
}
unlock {
if (!--count ) {
lock.release(); // throw()-nothing
tsd::set(&key, false);
}
}
Note that if you need "posix safety" with respect to
unlock/destroy of this lock, "tsd::set(&key, false);"
shall be done before "lock.release();" (and of course
the underlying nonrecursive lock itself shall be safe
with respect to unlock/destroy).
regards,
alexander.
SenderX
>
> SenderX wrote:
> [...]
> > ObjA.Lock();
> > ObjB.Lock();
> > < critical section >
> > ObjB.Unlock();
> > ObjA.Unlock();
> >
> > That will deadlock all over the place!
>
> It won't deadlock if the hidden lock ("shared" under the cover by
> ObjA and ObjB) is recursive.
I seem to remember a deadlock occurring because of lock-ordering w.r.t.
different objects on other threads getting hash collisions...
Once the lock-ordering gets messed up, your kinda screwed...
> > The hash'd lock API in my library uses per-thread flags...
>
> Per-thread flags can be used to turn nonrecursive locks into
> recursive ones.
Nice.