pthread_cond_wait does not relock mutex on release

pthread_cond_wait does not relock mutex on release

[Michael Labhard]

Don't no if anyone else has noticed this:  the pthread_cond_wait when
signalled does not lock the associated mutex again.  Thus the expected
output of the attached program should be

create Test
start
bar mutex lock and wait
foo mutex lock
foo mutex lock and signal condition then sleep...
foo mutex unlock                                            <------- Should
be here
bar mutex lock and wait released
bar mutex unlock
end
delete Test

But the actual output is:

create Test
start
bar mutex lock and wait
foo mutex lock
foo mutex lock and signal condition then sleep...
bar mutex lock and wait released
bar mutex unlock
foo mutex unlock                                          <------- But here
instead
end
delete Test


Test program:

#include <stdio.h>
#include <unistd.h>
#include <assert.h>
#include <pthread.h>


struct wait_cond {
 wait_cond();
 ~wait_cond();

 pthread_mutex_t  _mutex;
 pthread_cond_t  _cond;

 bool    _status;
};

wait_cond::wait_cond()
 : _status(false)
{
 int status = ::pthread_mutex_init(&_mutex, 0);
 if( 0 == status )
  status  = ::pthread_cond_init(&_cond, 0);
  if( 0 == status )
   _status = true;
 assert(_status);
}

wait_cond::~wait_cond()
{
 int status = ::pthread_cond_destroy(&_cond);
 assert(0==status);
 status = ::pthread_mutex_destroy(&_mutex);
 assert(0==status);
}


struct Test
{
 Test()
 : _pwait_condition( new wait_cond )
 {
  ::printf("create Test\n");
 }
 ~Test()
 {
  ::printf("delete Test\n");
  delete _pwait_condition;
 }

 void *foo(void*)
 {
  int status = ::pthread_mutex_lock(&(_pwait_condition->_mutex));
  ::printf("foo mutex lock\n");

  status = ::pthread_cond_signal(&(_pwait_condition->_cond));
  ::printf("foo mutex lock and signal condition then sleep...\n");

  ::sleep(1);
  status = ::pthread_mutex_unlock(&(_pwait_condition->_mutex));
  ::printf("foo mutex unlock\n");

  return 0;
 };

 void *bar(void*)
 {
  ::pthread_mutex_lock(&(_pwait_condition->_mutex));
  ::printf("bar mutex lock and wait\n");

  ::pthread_cond_wait(&(_pwait_condition->_cond)
    , &(_pwait_condition->_mutex));
  ::printf("bar mutex lock and wait released\n");

  ::pthread_mutex_unlock(&(_pwait_condition->_mutex));
  ::printf("bar mutex unlock\n");

  return 0;
 };

 wait_cond *_pwait_condition;
};

void * foo(void* pv)
{
 ((Test *)pv)->foo(0);
}

void * bar(void* pv)
{
 ((Test *)pv)->bar(0);
}

int main(char** argc, int argv)
{
 Test test;

 pthread_t t1, t2;

 ::printf("start\n");
 int status = ::pthread_create(&t2, 0, bar, &test);
 assert(0==status);
 status = ::pthread_create(&t1, 0, foo, &test);
 assert(0==status);
 ::sleep(5);
 ::printf("end\n");

 return 0;
}



--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

RE: pthread_cond_wait does not relock mutex on release

[Gerald S. Williams]

Independent of any other issues, the test case is wrong.
Specifically, pthread_cond_wait is allowed to spuriously
return (it's in the standard that way). You do have the
lock, though. That allows you to look at the variable and
determine if this was a spurious signal.

You have to wrap cond_wait with a check of the associated
variable (which would have to be set when you signalled
it).

In other words, this:

>  void *foo(void*)
>  {
>   int status = ::pthread_mutex_lock(&(_pwait_condition->_mutex));
>   ::printf("foo mutex lock\n");
> 
>   status = ::pthread_cond_signal(&(_pwait_condition->_cond));
>   ::printf("foo mutex lock and signal condition then sleep...\n");
> 
>   ::sleep(1);
>   status = ::pthread_mutex_unlock(&(_pwait_condition->_mutex));
>   ::printf("foo mutex unlock\n");
> 
>   return 0;
>  };
> 
>  void *bar(void*)
>  {
>   ::pthread_mutex_lock(&(_pwait_condition->_mutex));
>   ::printf("bar mutex lock and wait\n");
> 
>   ::pthread_cond_wait(&(_pwait_condition->_cond)
>     , &(_pwait_condition->_mutex));
>   ::printf("bar mutex lock and wait released\n");
> 
>   ::pthread_mutex_unlock(&(_pwait_condition->_mutex));
>   ::printf("bar mutex unlock\n");
> 
>   return 0;
>  };

should become something like this:

 void *foo(void*)
 {
  int status = ::pthread_mutex_lock(&(_pwait_condition->_mutex));
  ::printf("foo mutex lock\n");

  _pwait_condition->_status = false;
  status = ::pthread_cond_signal(&(_pwait_condition->_cond));
  ::printf("foo mutex lock and signal condition then sleep...\n");

  ::sleep(1);
  status = ::pthread_mutex_unlock(&(_pwait_condition->_mutex));
  ::printf("foo mutex unlock\n");

  return 0;
 };

 void *bar(void*)
 {
  ::pthread_mutex_lock(&(_pwait_condition->_mutex));
  ::printf("bar mutex lock and wait\n");

  while (_pwait_condition->_status) {
    ::pthread_cond_wait(&(_pwait_condition->_cond)
      , &(_pwait_condition->_mutex));
    ::printf("bar mutex lock and wait released\n");
  }

  ::pthread_mutex_unlock(&(_pwait_condition->_mutex));
  ::printf("bar mutex unlock\n");

  return 0;
 };

That could cause the results you are seeing. Or it could
be something much simpler. Calling printf simultaneously
from multiple threads can mess up the output. Mine was
really strange for this program. You might want to try
adding this to the top of your program:

#include <stdarg.h>

void SAFE_PRINTF(char *format,...)
{
    va_list vargs;
    static pthread_mutex_t printMutex;
    static int initialized = 0;

    /* OK as long as printf is called in single-threaded context first: */
    if (!initialized)
    {
        int status = pthread_mutex_init(&printMutex, 0);
        assert(0==status);
        initialized = 1;
    }

    pthread_mutex_lock(&printMutex);

    va_start(vargs,format);
    vprintf(format,vargs);
    va_end(vargs);

    pthread_mutex_unlock(&printMutex);
}

#define printf SAFE_PRINTF

I get the following output after changing printf:
 create Test
 start
 bar mutex lock and wait
 foo mutex lock
 foo mutex lock and signal condition then sleep...
 bar mutex lock and wait released
 foo mutex unlock
 bar mutex unlock
 end
 delete Test

That is still not exactly what you expected, but makes sense
since foo()'s printf is racing the two in bar(). (I.e., bar()
gets to run before foo()'s mutex_unlock() returns, then foo()
gets to run while bar() is unlocking the mutex again.)

-Jerry

-O Gerald S. Williams, 55A-134A-E   : mailto:gsw@agere.com O-
-O AGERE SYSTEMS, 6755 SNOWDRIFT RD : office:610-712-8661  O-
-O ALLENTOWN, PA, USA 18106-9353    : mobile:908-672-7592  O-


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

RE: pthread_cond_wait does not relock mutex on release

[Gerald S. Williams]

It's interesting that your system behaves so differently. (I
believe signals are supposed to interrupt condition variable
waits--perhaps you're system is being flooded by them?)

There are a few things you could try, including:

o Disabling signals in the created threads. To do this, you
  have to disable them in the parent, create the threads,
  then reenable them. It's generally worth it.

o Using a semaphore (you still have to check for a spurious
  wakeup due to a caught signal).


Here are some code snippets you might find helpful:


This creates threads, disabling signals (extracted from
Python's POSIX threads module):

    sigset_t oldmask, newmask;
    sigfillset(&newmask);
    pthread_sigmask(SIG_BLOCK, &newmask, &oldmask);
    pthread_create(...);
    pthread_sigmask(SIG_SETMASK, &oldmask, NULL);


These snippets should let you use semaphores to signal
between threads:


/*
 * As of February 2002, Cygwin thread implementations mistakenly report error
 * codes in the return value of the sem_ calls (like the pthread_ functions).
 * Correct implementations return -1 and put the code in errno. This supports
 * either.
 */
static int
fix_status(int status)
{
    return (status == -1) ? errno : status;
}

...


/* VARIABLES USED IN ALL SNIPPETS: */
    sem_t *sem;
    int status;


/* CREATE A SEMAPHORE */
    sem = (sem_t *)malloc(sizeof(sem_t));
    if (sem) {
        status = sem_init(sem,0,0); /* start "acquired" */
        if (status != 0) {
            free((void *)sem);
            sem = NULL;
        }
    }
    if (!sem) REPORT_ERROR();


/* DESTROY THE SEMAPHORE */
    if (sem) {
        status = sem_destroy(sem);
        if (status != 0) REPORT_ERROR();
        free(sem);
    }


/* WAIT FOR THE SIGNAL */
    do {
        status = fix_status(sem_wait(sem));
    } while (status == EINTR); /* Retry if interrupted by a signal */
    if (status != 0) REPORT_ERROR();


/* SIGNAL THE SEMAPHORE */
    status = sem_post(sem);
    if (status != 0) REPORT_ERROR();


I think the bug requiring the "fix_status" function has been
fixed, but you might find it easier to understand this way.
The "normal" way to wait for a semaphore is:
    do {
        status = sem_wait(sem);
    } while ((status == -1) && (errno == EINTR));


With semaphores you have to be careful to match the number of
sem_wait() and sem_post() calls (extra sem_post() calls would
cause future sem_wait()'s not to wait).


-Jerry

-O Gerald S. Williams, 55A-134A-E   : mailto:gsw@agere.com O-
-O AGERE SYSTEMS, 6755 SNOWDRIFT RD : office:610-712-8661  O-
-O ALLENTOWN, PA, USA 18106-9353    : mobile:908-672-7592  O-

> Robert and Gerald:
>
>          Both quite right.  Although adding the SAFE_PRINTF made no
> difference in the
> output, checking a condition value in a loop made all the difference.  
> Putting a printf in the loop revealed to my surprise that "spurious wakeups"
> were occurring thousands of times per second.  I had naively assumed that a
> condition would stay set until signalled.  Now I'm led to wonder if a
> condition variable is any performance improvement over a simple loop and
> short sleep.  Any thoughts?
>
>          Thanks very much for your time and effort.  It was a great help.
>
> -- Michael


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

RE: pthread_cond_wait does not relock mutex on release

[Robert Collins]

> -----Original Message-----
> From: Michael Labhard [mailto:ince@pacifier.com] 
> Sent: Wednesday, April 17, 2002 10:51 PM
> To: cygwin@cygwin.com
> Subject: Re: pthread_cond_wait does not relock mutex on release
> 
> 
> Robert and Gerald:
> 
>          Both quite right.  Although adding the SAFE_PRINTF made no 
> difference in the 
> output, checking a condition value in a loop made all the 
> difference.  
> Putting a printf in the loop revealed to my surprise that 
> "spurious wakeups" 
> were occurring thousands of times per second.  I had naively 
> assumed that a 
> condition would stay set until signalled.  Now I'm led to wonder if a 
> condition variable is any performance improvement over a 
> simple loop and 
> short sleep.  Any thoughts?

Without knowing what you are trying to achieve, I can't comment. However: in multi-threaded programs, a condition variable will almost invariably be more efficient than a loop + sleep.

As for conditions staying set, that isn't the purpose of condition variables: they are designed to allow signalling between threads, to allow efficient use of cpu resources - and to avoid that loop+sleep approach.

Rob

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: pthread_cond_wait does not relock mutex on release

[Michael Labhard]

Robert and Gerald:

         Both quite right.  Although adding the SAFE_PRINTF made no 
difference in the 
output, checking a condition value in a loop made all the difference.  
Putting a printf in the loop revealed to my surprise that "spurious wakeups" 
were occurring thousands of times per second.  I had naively assumed that a 
condition would stay set until signalled.  Now I'm led to wonder if a 
condition variable is any performance improvement over a simple loop and 
short sleep.  Any thoughts?

         Thanks very much for your time and effort.  It was a great help.

-- Michael


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

RE: pthread_cond_wait does not relock mutex on release

[Robert Collins]

> -----Original Message-----
> From: Michael Labhard [mailto:ince@pacifier.com] 
> Sent: Wednesday, April 17, 2002 5:01 AM
> To: cygwin@cygwin.com
> Subject: pthread_cond_wait does not relock mutex on release
> 
> 
> Don't no if anyone else has noticed this:  the 
> pthread_cond_wait when signalled does not lock the associated 
> mutex again.  

This is incorrect. It does lock the mutex again.:

(from __pthread_cond_dowait which implemented pthread_cond_wait)

  (*themutex)->Lock ();
  if (last == true)
    (*cond)->mutex = NULL;
  if (pthread_mutex_lock (&(*cond)->cond_access))
    system_printf ("Failed to lock condition variable access mutex, this
%p", *c
ond);
  InterlockedDecrement (&((*themutex)->condwaits));
  if (pthread_mutex_unlock (&(*cond)->cond_access))
    system_printf ("Failed to unlock condition variable access mutex,
this %p",
*cond);

  return rv;
}

Chances are your test app is buggy, or you are making one or more
assumptions about the mutex type (ie recursiveness etc) that are
incorrect.

Rob

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/