If you unlock an already unlocked mutex, is the behavior undefined?

Question

If you unlock an already unlocked mutex, is the behavior unsafe, safe, or undefined?

The purpose of the question is related to the following code, where I don't know if it would be better to unlock the mutexes within the if block, or just outside the if block.

    // This chunk of code makes dual locking semi-autonomous.
    int c_lckd = 0, q_lckd = 0;
    if (pthread_mutex_trylock(&crunch_mutex) == 0) c_lckd = 1;
    if (pthread_mutex_trylock(&queue_mutex) == 0) q_lckd = 1;
    if (q_lckd && !c_lckd) { QUEUE_UNLOCK; q_lckd = 0; }
    else if (c_lckd && !q_lckd) { CRUNCH_UNLOCK; c_lckd = 0; }

    if (c_lckd && q_lckd) {
      printf("cr = %d, max = %d, cnt = %d\n",
        crunching, max_crunching, queue_count(conn_queue));
      if (crunching < max_crunching && queue_count(conn_queue)) {
        pthread_t tid =
          pthread_create(
            &tid,
            NULL,
            crunch_conn,
            (void *)queue_dequeue(conn_queue)
          );
        crunching++;
      }
      CRUNCH_UNLOCK QUEUE_UNLOCK
    }

Thanks, Chenz

Answer 1

You don't need to do it that way. Try this:

    // This chunk of code makes dual locking semi-autonomous.
int c_lckd = 0, q_lckd = 0;
if (pthread_mutex_trylock(&crunch_mutex) == 0) c_lckd = 1;
if (pthread_mutex_trylock(&queue_mutex) == 0) q_lckd = 1;

if (c_lckd && q_lckd) {
  printf("cr = %d, max = %d, cnt = %d\n",
    crunching, max_crunching, queue_count(conn_queue));
  if (crunching < max_crunching && queue_count(conn_queue)) {
    pthread_t tid =
      pthread_create(
        &tid,
        NULL,
        crunch_conn,
        (void *)queue_dequeue(conn_queue)
      );
    crunching++;
  }

}

if (q_lckd) { QUEUE_UNLOCK; q_lckd = 0; }
if (c_lckd) { CRUNCH_UNLOCK; c_lckd = 0; }

It's a little easier to follow and doesn't risk trying to unlock an unlocked mutex.

Answer 2

In general, for questions like this, the documentation is the best source of information. Different mutexes may behave differently, or there may be options on a single mutex which cause it to behave different (such as in the case of recursively acquiring a mutex on a single thread).

Answer 3

For pthreads it will result in undefined behaviour. From the man page for pthread_mutex_unlock:

Calling pthread_mutex_unlock() with a mutex that the calling thread does not hold will result in undefined behavior.

Other mutexes will have their own behviour. As others have stated, it's best to read the manual for whichever mutex you're using.

Answer 4

As Glen noted, you get undefined behaviour if you attempt to unlock an unlocked mutex - don't try it. Debugging threads is hard enough without invoking undefined behaviour too.

More importantly, the coding style is a little unusual - since you aren't going to do anything unless you get both locks, code accordingly:

if (pthread_mutex_trylock(&crunch_mutex) == 0)
{
    if (pthread_mutex_trylock(&queue_mutex) == 0)
    {
        printf("cr = %d, max = %d, cnt = %d\n",
               crunching, max_crunching, queue_count(conn_queue));
        if (crunching < max_crunching && queue_count(conn_queue))
        {
            pthread_t tid;
            int rc = pthread_create(&tid, NULL,
                               crunch_conn, (void *)queue_dequeue(conn_queue));
            if (rc != 0)
            {
                // Error recovery
                // Did you need what was returned by queue_dequeue()
                // to requeue it, perhaps?
            }
            else
            {
                crunching++;
                // Do something with tid here?
            }
        }
        QUEUE_UNLOCK;
    }
    CRUNCH_UNLOCK;
}

This avoids the 'did I do it' variables; it is also instantly clear that as long as the unlock macros do what is expected (and there are no stray exceptions or setjmps around), that the mutexes that are locked are unlocked. It also avoids wasting energy on locking the queue mutex when the crunch mutex isn't available - but that's a minor issue compared to the added clarity.