possible problem with pthread mutexes, looking for opinions
Chris Friesen
wasn't missing something.
1)We have instrumented code of the following form:
lock_wrapper:
pthread_mutex_lock()
if the lock succeeded
lockcounter++
else
fail noisily
unlock_wrapper:
unlockcounter++
if (unlockcounter != lockcounter)
assert(0)
pthread_mutex_unlock()
I can think of three ways for the assertion to happen: 1) a thread that
isn't the owner is calling the unlock wrapper, or 2) there is a
compiler/pthread bug such that another thread is calling lock_wrapper
and the lockcounter increment is being hoisted before the lock, 3) the
underlying lock implementation is buggy.
Did I miss anything?
2) We have code that does the usual
pthread_mutex_lock()
...do stuff..
pthread_mutex_unlock()
and the unlock is occasionally failing with EPERM. Assuming that we're
not doing any locking from signal handlers, and that the lock call
succeeded, is there any other explanation other than a
pthread/compiler/kernel bug?
Thanks,
Chris
Chris M. Thomasson
news:gM6dneXMtegenbrX...@posted.sasktel...
> I've received reports of code failing, and I wanted to make sure that I
> wasn't missing something.
>
>
> 1)We have instrumented code of the following form:
>
> lock_wrapper:
> pthread_mutex_lock()
> if the lock succeeded
> lockcounter++
> else
> fail noisily
>
> unlock_wrapper:
> unlockcounter++
> if (unlockcounter != lockcounter)
> assert(0)
> pthread_mutex_unlock()
>
>
> I can think of three ways for the assertion to happen: 1) a thread that
> isn't the owner is calling the unlock wrapper,
Perhaps. That would be a buggy application, and not PThreads fault. Have you
inserted a breakpoint at the assertion and examined the state of the
applications threads?
> or 2) there is a
> compiler/pthread bug such that another thread is calling lock_wrapper
> and the lockcounter increment is being hoisted before the lock
Well, shi% does happen:
http://groups.google.com/group/comp.programming.threads/browse_frm/thread/63f6360d939612b3
Ouch!
> , 3) the
> underlying lock implementation is buggy.
>
> Did I miss anything?
When you write `fail noisily', what do you mean exactly? Are you calling
`abort()'? Has this ever happened? If so, what error code is it `EINVAL',
`EAGAIN' or perhaps `EDEADLK'? Are you using recursive and/or error checking
mutexs?
> 2) We have code that does the usual
>
> pthread_mutex_lock()
> ...do stuff..
> pthread_mutex_unlock()
>
> and the unlock is occasionally failing with EPERM.
That's a buggy application.
> Assuming that we're
> not doing any locking from signal handlers, and that the lock call
> succeeded, is there any other explanation other than a
> pthread/compiler/kernel bug?
Bugs in user application.
Chris Friesen
> "Chris Friesen" <cbf...@mail.usask.ca> wrote in message
> news:gM6dneXMtegenbrX...@posted.sasktel...
>>1)We have instrumented code of the following form:
>>
>>lock_wrapper:
>>pthread_mutex_lock()
>>if the lock succeeded
>>lockcounter++
>>else
>>fail noisily
>>I can think of three ways for the assertion to happen: 1) a thread that
>>isn't the owner is calling the unlock wrapper,
> Perhaps. That would be a buggy application, and not PThreads fault. Have you
> inserted a breakpoint at the assertion and examined the state of the
> applications threads?
I'm not directly involved in the testing. I haven't suggested manually
examining the state, but since they've already got a wrapper I suggested
tracking the owner themselves in the wrapper.
>>, 3) the
>>underlying lock implementation is buggy.
>>
>>Did I miss anything?
> When you write `fail noisily', what do you mean exactly?
If pthread_mutex_lock() returns nonzero, the wrapper calls assert(0).
In the error path that is being seen, this assertion is not being hit
but the one in the unlock case is.
> Are you using recursive and/or error checking
> mutexs?
No, the comparison of the unlock and lock counts is done only for
regular mutexes.
>>2) We have code that does the usual
>>
>>pthread_mutex_lock()
>>...do stuff..
>>pthread_mutex_unlock()
>>
>>and the unlock is occasionally failing with EPERM.
>
>
> That's a buggy application.
That's a bit harsh. It's possible (though maybe not likely) that a flaw
in the underlying locking implementation could cause this.
This following URL describes an assertion failing in
pthread_mutex_lock() because of a gcc compiler bug. A similar bug could
conceivably cause problems in pthread_mutex_unlock().
http://sourceware.org/bugzilla/show_bug.cgi?id=3328
Also, in this particular case the application is using C++. There is an
automatic variable in a particular scope. The mutex is passed to the
constructer, is locked in the constructor and unlocked in the
destructor. I've asked if there is any other places where that mutex is
locked/unlocked, and also whether the program aborts if the lock fails.
Chris
Chris M. Thomasson
> Chris M. Thomasson wrote:
>> "Chris Friesen" <cbf...@mail.usask.ca> wrote in message
>> news:gM6dneXMtegenbrX...@posted.sasktel...
>
>>>1)We have instrumented code of the following form:
>>>
>>>lock_wrapper:
>>>pthread_mutex_lock()
>>>if the lock succeeded
>>>lockcounter++
>>>else
>>>fail noisily
>
>
>>>I can think of three ways for the assertion to happen: 1) a thread that
>>>isn't the owner is calling the unlock wrapper,
>
>> Perhaps. That would be a buggy application, and not PThreads fault. Have
>> you
>> inserted a breakpoint at the assertion and examined the state of the
>> applications threads?
>
> I'm not directly involved in the testing. I haven't suggested manually
> examining the state, but since they've already got a wrapper I suggested
> tracking the owner themselves in the wrapper.
That should help...
>>>2) We have code that does the usual
>>>
>>>pthread_mutex_lock()
>>>...do stuff..
>>>pthread_mutex_unlock()
>>>
>>>and the unlock is occasionally failing with EPERM.
>>
>>
>> That's a buggy application.
>
> That's a bit harsh. It's possible (though maybe not likely) that a flaw
> in the underlying locking implementation could cause this.
Well, it's a habit of mine to automatically assume that it must be a bug in
my application instead of casting doubt on the underlying native
implementation. It very well may be a bug in the impl, but I have my doubts
because you say that the assertion sometimes hit on the `unlockcounter' not
being equal to the `lockcounter' which means that either the counters are
not being confined within the critical-section, or the application is
bugged.
> This following URL describes an assertion failing in
> pthread_mutex_lock() because of a gcc compiler bug. A similar bug could
> conceivably cause problems in pthread_mutex_unlock().
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=3328
Have you tried dissembling the compiled code and validating that the
counters are being confined and/or replacing the PThread mutex with a custom
one and seeing if you can reproduce the problem? How about creating an
isolated test case and see if suffers from the assertion on
`pthread_mutex_unlock()'? How about trying to use atomic fetch-and-add to
mutate the `lockcounter'? What about manually inserting a compiler barrier
before each counter update?
> Also, in this particular case the application is using C++. There is an
> automatic variable in a particular scope. The mutex is passed to the
> constructer, is locked in the constructor and unlocked in the
> destructor. I've asked if there is any other places where that mutex is
> locked/unlocked, and also whether the program aborts if the lock fails.
Please try the following simple program I just quickly coded up and see if
anything asserts:
http://cpt.pastebin.com/f2f632b93
If your getting `counter out of sync!' error, well, if your on an IA-32, you
can try to replace the mutations of `mutex::m_count' with fetch-and-add:
______________________________________________________________________
typedef char static_assert[
sizeof(unsigned) == 32 / CHAR_BIT ? 1 : -1
];
__attribute__((always_inline))
static __inline__
unsigned
atomic_ia32_faa32(
unsigned volatile* self,
unsigned const addend
) {
unsigned ret;
__asm__ __volatile__(
"LOCK XADDL %2, %1;\n"
: "=&r" (ret),
"=m" (*self)
: "0" (addend)
: "memory",
"cc"
);
return ret;
}
______________________________________________________________________
Any thoughts?
Chris M. Thomasson
Chris M. Thomasson
news:YAFVl.2127$GD4...@newsfe15.iad...
> [...]
>
> Please try the following simple program I just quickly coded up and see if
> anything asserts:
>
> http://cpt.pastebin.com/f2f632b93
Using the *nasty* `union thread_id' hack aside for a moment, I make small
dyslexic mistake in a call to `UNEXPECTED()'. The code in question is in the
`mutex::unlock()' procedure:
________________________________________________________________________
void unlock() {
if (! (m_count % 2)) {
UNEXPECTED(("`mutex::unlock()' counter out of sync!"));
}
thread_id self;
self.tid = pthread_self();
if (! pthread_equal(m_owner.tid, self.tid)) {
UNEXPECTED(("`mutex::unlock()' thread(%u) is NOT owner! thread(%u)
is!",
m_owner.id, self.id));
}
++m_count;
#if ! defined (NDEBUG)
thread_id dbg_temp_id = m_owner;
unsigned dbg_temp_count = m_count;
#endif
if (pthread_mutex_unlock(&m_mutex)) {
UNEXPECTED(("`pthread_mutex_unlock()' has failed!"));
}
DBG_PRINTF(("Thread(%u) has unlocked mutex (%p): %u\n",
dbg_temp_id.id, (void*)this, dbg_temp_count));
}
________________________________________________________________________
Its in the second call to `UNEXPECTED()'. It should look like:
UNEXPECTED(("`mutex::unlock()' thread(%u) is NOT owner! thread(%u)
is!",
self.id, m_owner.id));
instead. Here is fix:
http://cpt.pastebin.com/f38db00f3
Sorry about that.
Hallvard B Furuseth
> 1)We have instrumented code of the following form:
> I can think of three ways for the assertion to happen: 1) a thread that
> isn't the owner is calling the unlock wrapper, or 2) there is a
> compiler/pthread bug such that another thread is calling lock_wrapper
> and the lockcounter increment is being hoisted before the lock, 3) the
> underlying lock implementation is buggy.
>
> Did I miss anything?
4) A thread calls the lock wrapper when it already owns the mutex.
5) Destroying the mutex while it's still in use, either via
pthread_mutex_destroy() or by freeing the memory which contains it.
6) Using the lock uninitialzed.
7) Using pthread_cond_wait() - it unlocks/locks the mutex without
going via the wrapper.
When I wanted pthread wrappers to track such problems, I wrapped
pthread_mutex & co in structs that included a dummy memory pointer.
Malloc it at init, access before use, free at destroy. Allowed me to
run it under valgrind and have it report where detected problems were
coming from - either as memory leaks or as invalid memory accesses.
The code may suffer from bitrot. It does suffer from heavy macro magic,
as it's a wrapper around a wrapper thread implementations, but anyway:
http://www.openldap.org/devel/cvsweb.cgi/libraries/libldap_r/thr_debug.c
http://www.openldap.org/devel/cvsweb.cgi/libraries/libldap_r/ldap_thr_debug.h
http://www.openldap.org/devel/cvsweb.cgi/include/ldap_pvt_thread.h
http://www.openldap.org/devel/cvsweb.cgi/include/ldap_int_thread.h
Generally, read 'ldap_pvt_thread_' as 'pthread_' (or the exported
interface resembling it), 'ldap_int_thread_' as internal functions
or wrappers, 'ldap_debug_thread_' as wrappers around wrappers when
some of that couldn't be hidden by macros.
--
Hallvard
Chris Friesen
> BTW, what platform and architecture is this application failing under?
glibc/linux. The first problem I brought up is failing on mips (32bit
userspace, 64bit kernel). The second is showing up on both mips and x86
(also 32-bit userspace, 64bit kernel).
To add complexity to the mix, glibc and the kernel have been patched
(not by me) to support priority inheritance since the versions in
question are old enough that they didn't originally support it.
In any case, I have some more debugging information now and the app is
running with added instrumentation.
Chris
Chris M. Thomasson
> Chris M. Thomasson wrote:
>> BTW, what platform and architecture is this application failing under?
>
> glibc/linux. The first problem I brought up is failing on mips (32bit
> userspace, 64bit kernel). The second is showing up on both mips and x86
> (also 32-bit userspace, 64bit kernel).
I know both platforms; okay...
> To add complexity to the mix, glibc and the kernel have been patched
> (not by me) to support priority inheritance since the versions in
> question are old enough that they didn't originally support it.
So, your `SCHED_OTHER' by default, on a global scale? Humm... Interesting...
> In any case, I have some more debugging information now and the app is
> running with added instrumentation.
Please enlighten me!!!!!!!
Seriously! I love to be educated!!!
Chris M. Thomasson
"Hallvard B Furuseth" <h.b.fu...@usit.uio.no> wrote in message
news:hbf.2009...@bombur.uio.no...
Great! I break my brain looking through macros, but the idea of forcing all
allocations of locks through a "std" allocator (e.g., `malloc()'), well,
that's okay. Valgrind to the rescue. and it just adds another lay that can
be deciphered by a third party tool. Seems to be a-okay with me.
BTW, PLEASE TAKE A LOOK AT
RELACY!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
http://groups.google.com/group/relacy
http://groups.google.com/group/relacy
Chris M. Thomasson
[...]
> Great! I break my brain looking through macros, but the idea of forcing
> all allocations of locks through a "std" allocator (e.g., `malloc()'),
> well, that's okay. Valgrind to the rescue. and it just adds another lay
`layer' of course!
> that can be deciphered by a third party tool. Seems to be a-okay with me.
> BTW, PLEASE TAKE A LOOK AT
> RELACY!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
>
>
>
>
>
> http://groups.google.com/group/relacy
>
>
>
> http://groups.google.com/group/relacy
>
>
>
> http://groups.google.com/group/relacy
>
>
>
> http://groups.google.com/group/relacy
Relacy can cut right through memory corruption, and eat leaks for breakfast,
lunch and dinner in its sleep! Yes, you can convert C to Relacy... Well, you
have to be a programmer!
;^o