4

I have a thread local singleton class which looks a little like this:

// UserActionManager.hh

class UserActionManager
{
public:
    static UserActionManager* GetUserActionManager();

    ~UserActionManager() { std::cout << "UAM destructor called.\n"; }

    static thread_local std::unique_ptr<UserActionManager> theInstance;
};

// UserActionManager.cc

thread_local std::unique_ptr<UserActionManager> UserActionManager::theSafeInstance = nullptr;

UserActionManager*
UserActionManager::
GetUserActionManager()
{
    if (nullptr == theInstance)
    {
        theInstance.reset(new UserActionManager());
        std::cout << "theInstance = " << theInstance.get() << "\n";
    }

    return theInstance.get();
}

I need to use a dynamic allocation for the instance because the UserActionManager must be created after other steps have been completed in my program.

When I run the program, it works fine until program exit. I get an error like this:

simple(57724,0x10b647000) malloc: *** error for object 0x7fc64ce79460: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
simple(57724,0x10d64d000) malloc: *** error for object 0x7fc64ce7b140: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
simple(57724,0x10c64a000) malloc: *** error for object 0x7fc64ce7b8e0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

 *** Break *** illegal instruction

 *** Break *** illegal instruction

 *** Break *** illegal instruction

If I run the program in lldb, I get this message, which tells me the crash is happening on the default delete call for my thread_local UserActionManager instance.

// lldb output

Process 57201 stopped
* thread #2: tid = 0x8d5d1, 0x0000000105a2eed7 libGex.dylib`std::default_delete<gex::ua::UserActionManager>::operator()(gex::ua::UserActionManager*) const + 27, stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
    frame #0: 0x0000000105a2eed7 libGex.dylib`std::default_delete<gex::ua::UserActionManager>::operator()(gex::ua::UserActionManager*) const + 27
libGex.dylib`std::default_delete<gex::ua::UserActionManager>::operator()(gex::ua::UserActionManager*) const + 27:
-> 0x105a2eed7:  movq   (%rax), %rax
   0x105a2eeda:  addq   $0x48, %rax
   0x105a2eede:  movq   (%rax), %rax
   0x105a2eee1:  movq   -0x10(%rbp), %rdx

However, if you look at the pointers printed by the code for the thread local instances and the pointers which are being freed without being allocated, they are not the same:

// Pointers printed by GetUserActionManager():

0x7fc650948300
0x7fc64a4de670
0x7fc650b33480
0x7fc650c19f90

// Pointers causing crash:

0x7fc64ce79460
0x7fc64ce7b140
0x7fc64ce7b8e0

What is going on here? I have used this code in the past without issue. Now I am using g++ 4.9.1 on MacOSX and c++14 and getting this crash. The destructor for the UserActionManager is never being called, and the pointer the default delete is trying to free is different from the one it should have. I have grepped the code, and this is only reference to theInstance is in the static method that creates it, so I don't think this is a side-effect from elsewhere. The code works fine if I use a raw pointer for theInstance.

Any suggestions on how to fix this problem? Thanks!

EDIT:

I just found this:

Why global or static object can lead to crash when program exit?

The UserActionManager is now part of a library and the program "simple" is linking to it. Previously (when this was working) I was compiling the code all as one piece. Could this be causing the issue?"

Community
  • 1
  • 1
user487100
  • 918
  • 1
  • 11
  • 22
  • Your `GetUserActionManager` is fishy: the initialization of `theInstance` is done (nullptr) and it might need a guard. You may remove the `static thread_local` member and put it into the function `GetUserActionManager` with a proper initialization (no nullptr) –  Oct 15 '14 at 19:53
  • Thanks! Can you clarify what you mean about needing a guard? I think nullptr is an acceptable initialization for a unique_ptr. But I tried what you said, moving the variable out of the class and into the GetUserActionManager() method, initializing it to std::unique_ptr(). Unfortunately I still get the same crash. – user487100 Oct 15 '14 at 20:11
  • You need a guard (lock on a mutex) for `reset(new UserActionManager())` - `theInstance = std::make_unique()` if avaliable –  Oct 15 '14 at 20:18
  • Why is that? Doesn't the fact that the variable is thread local protect it from asynchronous access? I thought only the single "thread local" thread could access thread local data. – user487100 Oct 15 '14 at 20:21
  • @DieterLücking The pointer is `thread_local` which scopes the pointers to threads. So, there's no race. – Jason Oct 15 '14 at 20:23
  • @user487100 Why are you using a `unique_ptr` instead of a naked pointer? – Jason Oct 15 '14 at 20:24
  • @Jason, I wanted the destructor for UserActionManager to be called when the program exited. This won't happen with a raw pointer. Here's a live example: http://ideone.com/KGzOWy (I couldn't use a static value UserActionManager because the UserActionManager can't be created at the beginning of the program without error.) – user487100 Oct 15 '14 at 20:34
  • @user487100 Are the threads detached? – Jason Oct 15 '14 at 21:23
  • @Jason No they are managed by another library and joined before program exit. But I don't actually handle the creation of threads, just use this and other classes to interact with the multi-threaded framework I am using. – user487100 Oct 15 '14 at 21:28
  • Have you tried adding a default_delete for the UserActionManager in your library, e.g. template <> class default_delete< UserActionManager> and provide the void operator()(UserActionManager* ); – Jens Munk Dec 06 '14 at 23:08

0 Answers0