Move Thread Name Mapping into ThreadFunc.

base/threading/thread.cc

Index: base/threading/thread.cc
diff --git a/base/threading/thread.cc b/base/threading/thread.cc
index 35e88a70569a2b6dfcae8122bab5290d57ed140a..123c9778bfd156d0d46fc62332307e244e23fc11 100644
--- a/base/threading/thread.cc
+++ b/base/threading/thread.cc
@@ -66,7 +66,6 @@ Thread::Thread(const char* name)
 
 Thread::~Thread() {
   Stop();
-  ThreadIdNameManager::GetInstance()->RemoveName(thread_id_);
 }
 
 bool Thread::Start() {
@@ -121,6 +120,8 @@ void Thread::Stop() {
   //
   PlatformThread::Join(thread_);
 
+  ThreadIdNameManager::GetInstance()->RemoveName(thread_id_);

[jar (doing other things), 2013/05/14 16:49:10]

Given the problem you had... this solution is still racy :-(.

I suspect you need more state in the name manager, such as a back-pointer to
this instance... and then the delete name has to be careful to delete only the
name with the correct back pointer.

This does create a quandry, when someone is looking up the name :-/.

Perhaps what you need is a "most recently registered" (which is surely the most
current) as well as a reference count (so you only delete when the last
RemoveName is called.

...or am I missing something in the logic?

[dsinclair, 2013/05/14 17:00:17]

You're right, there is still a race here.

Looking at the code, the simplest solution seems like it would be to move this
line up above the StopSoon() call. That way, the thread is shutting down, but
hasn't executed the code to quit the message loop so the thread_id should still
be allocated to this thread. (Unless I'm failing to understand the code flow
here.)

Does that make sense? This way there should only be one instance of any
thread_id getting a name, so we don't have to change the manager.

[jar (doing other things), 2013/05/14 19:55:06]

I don't fully grok the use case... but I suspect you don't want to unregister
while the thread is active... and may be running task after task after task :-(.
 That is sadly the case before and after StopSoon().

You could do the unregister in the actual thread shutdown code, such as when the
message loop unwinds.  That would probably get rid of the race, and be safe. You
can't start a new thread until you "know" that the old thread using the same
name has terminated... and the decision to terminate is made in the message loop
(or message pump) code.

+
   // The thread should NULL message_loop_ on exit.
   DCHECK(!message_loop_);