28
class AAA
{
    ...
    ~AAA()
    {
        pthread_mutex_lock( &m_mutex );
        pthread_mutex_destroy( &m_mutex );
    }
}

Question> I saw this code somewhere in a project. Is it good practice to do so? Or it is undefined behavior to lock a mutex before destroying it?

indiv
  • 17,306
  • 6
  • 61
  • 82
q0987
  • 34,938
  • 69
  • 242
  • 387
  • 12
    If you feel the need to lock it, it should imply that another thread could try to ask for it. What would happen to that thread when the mutex was destroyed?. – Jan Petter Jetmundsen Nov 07 '14 at 19:34
  • @JanPetterJetmundsen, I didn't write the code and need to understand the reason behind it. – q0987 Nov 07 '14 at 19:36
  • 1
    It's undefined. @JanPetterJetmundsen is right. It's not a good practice to lock the mutex before destroy it, even though in most implementations it does not do anything wrong. – Peter Li Nov 07 '14 at 19:41
  • @Gabriel How can that other question be a duplicate if its answers seem to be the exact opposite of the ones provided below? – Roddy of the Frozen Peas Nov 07 '14 at 23:50
  • 2
    @JanPetterJetmundsen this code strikes me as _so preposterously bad_ that I'm (hoping) there's a reason. I'd like to know what the person who actually wrote the code was thinking--this kind of apparent error takes _effort_. – geometrian Nov 08 '14 at 00:27
  • @imallet "I want to destroy my mutex. But what if someone else tries to use it? Better make the destruction thread-safe. And what luck! I already have the perfect mutex for that." – Dan Getz Nov 08 '14 at 13:18
  • Looking down the related questions list, I see a lot of people finding themselves in similar situations, where they want to destroy a mutex without truly being done with it. Sounds like a common mistake. – Dan Getz Nov 08 '14 at 13:21

2 Answers2

35

It strikes me as utterly terrible practice.

from http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_destroy.html

It shall be safe to destroy an initialized mutex that is unlocked. Attempting to destroy a locked mutex results in undefined behavior.

so this code guarantees undefined behavior and needs to be fixed.

camelccc
  • 2,847
  • 8
  • 26
  • 52
  • 3
    For completeness, the up-to-date man-page: http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_destroy.html – alk Nov 07 '14 at 19:57
  • Also note that according to that man page, attempting to *lock* a *destroyed* mutex also results in undefined behavior. (If there's any reason to lock this mutex, that means another thread might try to lock it, too.) – Dan Getz Nov 08 '14 at 13:15
6

This link says its undefined behavior.

Maybe from where you saw this code, the original coder wanted to destroy the mutex and might have thought that if he/she would be able to lock that mutex, then that means it's unlocked somewhere else by some important thread, and thus he can delete it.

But it's implemented incorrectly.

ragingasiancoder
  • 616
  • 6
  • 17
Rupesh Yadav.
  • 894
  • 2
  • 10
  • 24