Skip to content

Commit c075ecd

Browse files
committed
Fix a think in Mutex::Locker::Locker(pthread_mutex_t *) Really should lock the mutex handed in, not the m_mutex_ptr that you've set to NULL...
Rework the Host.cpp::ThreadNameAccessor to use ThreadSafeSTLMap - we've got it so we might as well use it. Also works around a problem with the Mutex::Locker class raising fallacious asserts in debug mode when used with pthread_mutex_t's that weren't backed by Mutex objects. llvm-svn: 156193
1 parent ce52ca1 commit c075ecd

File tree

2 files changed

+32
-19
lines changed

2 files changed

+32
-19
lines changed

lldb/source/Host/common/Host.cpp

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "lldb/Core/Error.h"
1515
#include "lldb/Core/Log.h"
1616
#include "lldb/Core/StreamString.h"
17+
#include "lldb/Core/ThreadSafeSTLMap.h"
1718
#include "lldb/Host/Config.h"
1819
#include "lldb/Host/Endian.h"
1920
#include "lldb/Host/FileSpec.h"
@@ -610,38 +611,51 @@ Host::ThreadJoin (lldb::thread_t thread, thread_result_t *thread_result_ptr, Err
610611
return err == 0;
611612
}
612613

614+
// rdar://problem/8153284
615+
// Fixed a crasher where during shutdown, loggings attempted to access the
616+
// thread name but the static map instance had already been destructed.
617+
// So we are using a ThreadSafeSTLMap POINTER, initializing it with a
618+
// pthread_once action. That map will get leaked.
619+
//
620+
// Another approach is to introduce a static guard object which monitors its
621+
// own destruction and raises a flag, but this incurs more overhead.
622+
623+
static pthread_once_t g_thread_map_once = PTHREAD_ONCE_INIT;
624+
static ThreadSafeSTLMap<uint64_t, std::string> *g_thread_names_map_ptr;
625+
626+
static void
627+
InitThreadNamesMap()
628+
{
629+
g_thread_names_map_ptr = new ThreadSafeSTLMap<uint64_t, std::string>();
630+
}
631+
613632
//------------------------------------------------------------------
614633
// Control access to a static file thread name map using a single
615634
// static function to avoid a static constructor.
616635
//------------------------------------------------------------------
617636
static const char *
618637
ThreadNameAccessor (bool get, lldb::pid_t pid, lldb::tid_t tid, const char *name)
619638
{
639+
int success = ::pthread_once (&g_thread_map_once, InitThreadNamesMap);
640+
if (success != 0)
641+
return NULL;
642+
620643
uint64_t pid_tid = ((uint64_t)pid << 32) | (uint64_t)tid;
621644

622-
static pthread_mutex_t g_mutex = PTHREAD_MUTEX_INITIALIZER;
623-
Mutex::Locker locker(&g_mutex);
624-
625-
typedef std::map<uint64_t, std::string> thread_name_map;
626-
// rdar://problem/8153284
627-
// Fixed a crasher where during shutdown, loggings attempted to access the
628-
// thread name but the static map instance had already been destructed.
629-
// Another approach is to introduce a static guard object which monitors its
630-
// own destruction and raises a flag, but this incurs more overhead.
631-
static thread_name_map *g_thread_names_ptr = new thread_name_map();
632-
thread_name_map &g_thread_names = *g_thread_names_ptr;
633-
634645
if (get)
635646
{
636647
// See if the thread name exists in our thread name pool
637-
thread_name_map::iterator pos = g_thread_names.find(pid_tid);
638-
if (pos != g_thread_names.end())
639-
return pos->second.c_str();
648+
std::string value;
649+
bool found_it = g_thread_names_map_ptr->GetValueForKey (pid_tid, value);
650+
if (found_it)
651+
return value.c_str();
652+
else
653+
return NULL;
640654
}
641-
else
655+
else if (name)
642656
{
643657
// Set the thread name
644-
g_thread_names[pid_tid] = name;
658+
g_thread_names_map_ptr->SetValueForKey (pid_tid, std::string(name));
645659
}
646660
return NULL;
647661
}

lldb/source/Host/common/Mutex.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,7 @@ Mutex::Locker::Locker (Mutex* m) :
132132
Mutex::Locker::Locker (pthread_mutex_t *mutex_ptr) :
133133
m_mutex_ptr (NULL)
134134
{
135-
if (m_mutex_ptr)
136-
Lock (m_mutex_ptr);
135+
Lock (mutex_ptr);
137136
}
138137

139138
//----------------------------------------------------------------------

0 commit comments

Comments
 (0)