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

score 29 · Accepted Answer · edited Nov 22 '09 at 14:37

29

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.

edited Nov 22 '09 at 14:37

Jonathan Leffler

730,956
141
904
1,278

answered Nov 22 '09 at 14:22

Glen

21,816
3
61
76

The result depends upon the type of the mutex as per the man page whose link you have kindly provided. The behaviour as per the page is either 'undefined' or an error is returned. – work.bin Dec 09 '15 at 20:25

Jonathan Leffler · Answer 2 · 2009-11-22T14:58:49.773

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.

score 2 · Answer 3 · answered Nov 22 '09 at 14:12

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).

Michael Kohne · Answer 4 · 2009-11-22T17:39:37.153

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.

edited Nov 22 '09 at 17:39

answered Nov 22 '09 at 14:06

Michael Kohne

11,888
3
47
79

thanks!, that does clean it up a bit. Although I'd still be interested if anyone knows if the "unlocking and unlocked" pattern is explicitly stated in the POSIX standard. – Crazy Chenz Nov 22 '09 at 14:09
Even if it's specified in POSIX, I'd avoid doing it - my experience leads me to keep well away from the odder corner conditions like this one - even if it's specified, I've been burned too many times by less-then compliant libraries to be comfortable trying it. – Michael Kohne Nov 22 '09 at 17:42
How do you figure race conditions? c_lckd and q_lckd are on the local stack. Any concurrent call to this code will have it's own flags. Thus, no other thread can possibly care about these flags, thus no race conditions. (If those flags were shared, this couldn't work at all anyway, due to the trylocks) – Michael Kohne Nov 22 '09 at 18:49

score 0 · Answer 5 · answered Dec 08 '15 at 11:47

A mutex unlock should be done in a thread only if the same mutex is locked earlier in the same thread. All other cases are undefined behviour as per man page.

If the mutex type is PTHREAD_MUTEX_DEFAULT, attempting to recursively lock the mutex results in undefined behaviour. Attempting to unlock the mutex if it was not locked by the calling thread results in undefined behaviour. Attempting to unlock the mutex if it is not locked results in undefined behaviour.

score 0 · Answer 6 · answered Jul 04 '16 at 16:33

Try it. This is code working correct.

// Mutex is not busy
if(pthread_mutex_trylock(&object->mtx) == 0) {
    if(pthread_mutex_unlock(&object->mtx)!=0) {
        perror("ERRN: pthread_mutex_unlock:");
    }
}
// Mutex is already busy
else {
    if(pthread_mutex_unlock(&object->mtx)!=0) {
        perror("ERRN: pthread_mutex_unlock:");
    }
}

// In this point -- we correctly unlocked mutex.

if(pthread_mutex_destroy(&object->mtx) != 0) {
    perror("ERRN: pthread_mutex_destroy:");
}

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

6 Answers6

Linked