Frank Cusack <fcusack@fcusack.com> writes:

>On 08 Feb 2008 20:23:04 GMT Casper H.S. Dik <Casper.Dik@Sun.COM> wrote:
>> No; perhaps he just didn't link with -lpthread so all therad
>> functions (in S9 and before) are noops.

>annoying. what was the rationale for including noops in libc as
>opposed to letting that kind of error be detected at link time?

So that you can build thread-safe libraries which use no-op
functions in apps not linked w/ libthread and which use the
real function when linking with libthread.

Not entirely sure why pthread_create() was included in this, but
all the mutex and related functions make perfect sense.

Fixed in Solaris 10, BTW, where it's threaded all the way.

Casper
--
Expressed in this posting are my opinions. They are in no way related
to opinions held by my employer, Sun Microsystems.
Statements on Sun products included here are not gospel and may
be fiction rather than truth.

On Sat, 09 Feb 2008 11:13:42 +1300 Ian Collins <ian-news@hotmail.com> wrote:
> I interpreted the comment as "fiddling around inside the pthread_mutex_t
> object". There's nothing in the code that would prevent it compiling on
> Solaris. Whether it is necessary or not isn't the issue.

crossed streams, sorry 'bout that

-lpthread and -lthread are both used in linking. I debugged a little
bit more by printing some info:

void
posixMutex::lock()
{
if (m_dontLock)
return;

threadID thisthreadID = getThisThreadID();

cerr << "Starting to lock mutex " << thisthreadID << " locked = "
<< isProcessLocked() << " by " << lockedBy() << endl;

if (isProcessLocked() && lockedBy() == thisthreadID)
{
cerr << "can not lock " << thisthreadID << " as " <<
m_state.m_lockCount << endl;
m_state.m_lockCount++;
return;
}

cerr << "Before locking " << thisthreadID << endl;
int retVal = pthread_mutex_lock(&m_mutex);

/* int retVal;
while (retVal = pthread_mutex_trylock(&m_mutex))
sleep(0.2);
*/
cerr << "After locking " << thisthreadID << endl;

if (retVal != 0) {
throw exception_INTERNAL("Fail to mutex locking");
}

// This is the correct order to set these in to avoid a race-
condition
// with the test above the lock call...

m_state.m_lockedBy = thisthreadID;
m_state.m_lockCount = 1;
m_lockingProcessID = getThisProcessID();
}

void
posixMutex::unlock()
{
if (m_dontLock)
return;

threadID thisthreadID = getThisThreadID();

cerr << "unlocking mutex " << thisthreadID << " locked = " <<
isProcessLocked() << " by " << lockedBy() << endl;
if (isProcessLocked() && lockedBy() == getThisThreadID())
{
if (--m_state.m_lockCount > 0)
return;
}
else
{
m_state.m_lockCount = 0;
}

m_lockingProcessID = 0;
m_state.m_lockedBy = 0;

int retVal = pthread_mutex_unlock(&m_mutex);

if (retVal != 0)
{
throw exception_INTERNAL("fail to unlock the mutex");
}
}


The output:

Starting to lock mutex 4 locked = 0 by 0
Before locking 4
After locking 4
unlocking mutex 4 locked = 1 by 4
Starting to lock mutex 4 locked = 0 by 0
Before locking 4
After locking 4
unlocking mutex 4 locked = 1 by 4
ERROR -- call to pthread_mutex_lock() -- called on locked mutex
ERROR -- call to pthread_mutex_lock() -- called on locked mutex

****************************
It seems only one thread was doing the lock - unlock, and each lock-
unlock cycle was complete. Then where is the relock on locked mutex
coming from??

On Fri, 8 Feb 2008 15:26:50 -0800 (PST) Cheng <cheng.stillsea@gmail.com> wrote:
> -lpthread and -lthread are both used in linking. I debugged a little
> bit more by printing some info:
>
> void
> posixMutex::lock()
> {
....
> // This is the correct order to set these in to avoid a race-
> condition
> // with the test above the lock call...
>
> m_state.m_lockedBy = thisthreadID;
> m_state.m_lockCount = 1;
> m_lockingProcessID = getThisProcessID();

the comment is incorrect. there is NO ordering which either
introduces or avoids a race. show us isProcessLocked(). i bet
it merely checks m_lockingProcessID. if so, that's your race.

-frank

Here it is:

bool
posixMutex::isProcessLocked() const
{
return (m_lockingProcessID == getThisProcessID() &
m_state.m_lockCount != 0);
}

So what else should it check in your opinion? Thank you.

On Feb 8, 4:24*pm, Frank Cusack <fcus...@fcusack.com> wrote:
> On Fri, 8 Feb 2008 15:26:50 -0800 (PST) Cheng <cheng.still...@gmail.com> wrote:
>
> > -lpthread and -lthread are both used in linking. I debugged a little
> > bit more by printing some info:
>
> > void
> > posixMutex::lock()
> > {
> ...
> > * * // This is the correct order to set these in to avoid a race-
> > condition
> > * * // with the test above the lock call...
>
> > * * m_state.m_lockedBy = thisthreadID;
> > * * m_state.m_lockCount = 1;
> > * * m_lockingProcessID = getThisProcessID();
>
> the comment is incorrect. *there is NO ordering which either
> introduces or avoids a race. *show us isProcessLocked(). *i bet
> it merely checks m_lockingProcessID. *if so, that's your race.
>
> -frank

Cheng wrote:

[please don't top-post]

> On Feb 8, 5:02 pm, Frank Cusack <fcus...@fcusack.com> wrote:
>> On Fri, 8 Feb 2008 15:26:50 -0800 (PST) Cheng <cheng.still...@gmail.com> wrote:
>>
>>> -lpthread and -lthread are both used in linking.
>> BTW, you should use only one or the other. I suggest -lpthread unless
>> you specifically need some solaris threads functionality. I think
>> (not 100% sure) using both effectively only uses the first one on
>> your link command line.
>>
> I see what you mean. And it works after taking care of the data race
> problem (by disabling the recursive lock checking). Thank you very
> much!
> However, two questions still remain:
>
> 1. What difference between Linux and Solaris threading mechanisms
> causes this? Why the data race was not a problem in Linux?

Two different scheduler implementations. You didn't say what hardware
you were running on, was one singe and one multi-core? Odds are you
could find a Linux box that hits the problem.

> 2. Is there an easy solution to enable recursive checking (although it
> is not really necessary) while avoiding data race? I have thought
> about it but can't find a way to pass the mutex ownership info between
> threads...
>
If you must do this, use a recursive mutex.

--
Ian Collins.