Close duplicated handle on windows.

base/threading/platform_thread_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/threading/platform_thread.h"
 
 #include "base/debug/alias.h"
 #include "base/debug/profiler.h"
 #include "base/logging.h"
 #include "base/threading/thread_id_name_manager.h"
@@ skipping 37 matching lines @@
   bool joinable;
 };
 
 DWORD __stdcall ThreadFunc(void* params) {
   ThreadParams* thread_params = static_cast<ThreadParams*>(params);
   PlatformThread::Delegate* delegate = thread_params->delegate;
   if (!thread_params->joinable)
     base::ThreadRestrictions::SetSingletonAllowed(false);
 
   /* Retrieve a copy of the thread handle to use as the key in the
    * thread name mapping. */

[rvargas (doing something else), 2013/12/02 20:19:37]

Use c++ style comments

[dsinclair, 2013/12/10 21:08:20]

Done.

   PlatformThreadHandle::Handle platform_handle;
   DuplicateHandle(

[jar (doing other things), 2013/11/28 19:01:22]

Is it worth checking the return value for success?

What will the CloseHandle() do if we failed here?

[rvargas (doing something else), 2013/12/02 20:19:37]

Yes, check the return value!. Create a win::ScopedHandle after success.

[dsinclair, 2013/12/10 21:08:20]

Done.

       GetCurrentProcess(),
       GetCurrentThread(),
       GetCurrentProcess(),
       &platform_handle,
       0,
       FALSE,
       DUPLICATE_SAME_ACCESS);
 
   ThreadIdNameManager::GetInstance()->RegisterThread(
       platform_handle,
       PlatformThread::CurrentId());
 
   delete thread_params;
   delegate->ThreadMain();
 
   ThreadIdNameManager::GetInstance()->RemoveName(
       platform_handle,
       PlatformThread::CurrentId());
+
+  CloseHandle(platform_handle);

[zhaoqin, 2013/11/28 16:07:33]

Actually I am not sure this is the correct solution.

The handle is created  hereusing DuplicateHandle. Then the newly created handle
is passed to ThreadIdNameManager::RegisterThread and added into a list.
see src/base/threading/thread_id_name_manager.cc

void ThreadIdNameManager::RegisterThread(PlatformThreadHandle::Handle handle,
                                         PlatformThreadId id) {
  AutoLock locked(lock_);
  thread_id_to_handle_[id] = handle;
  thread_handle_to_interned_name_[handle] =
      name_to_interned_name_[kDefaultName];
}

So if you close the handle here, the reference in thread_id_to_handle_[id] will
be invalid.

[dsinclair, 2013/11/28 16:13:40]

Right, but the line immediately above:

  ThreadIdNameManager::GetInstance()->RemoveName(
       platform_handle,
       PlatformThread::CurrentId());

Removes the handle from the maps.

It's cleared from the thread_handle_to_interned_name_ map on line 100 and from
the thread_id_to_handle_ map on line 110 of thread_id_name_manager.cc.  

[zhaoqin, 2013/11/28 16:25:41]

I see, my bad.
In that case, should CloseHandle be called in ThreadIdNameManager::RemoveName
right after the handle is removed?

[dsinclair, 2013/11/28 16:31:07]

I don't think so, ThreadIdNameManager doesn't know the handle should be closed.
It just knows it shouldn't have the mapping to it anymore.

As well, how to close the handle as it's platform dependent.

+
   return NULL;
 }
 
 // CreateThreadInternal() matches PlatformThread::Create(), except that
 // |out_thread_handle| may be NULL, in which case a non-joinable thread is
 // created.
 bool CreateThreadInternal(size_t stack_size,
                           PlatformThread::Delegate* delegate,
                           PlatformThreadHandle* out_thread_handle) {
   unsigned int flags = 0;
@@ skipping 141 matching lines @@
     case kThreadPriority_RealtimeAudio:
       ::SetThreadPriority(handle.handle_, THREAD_PRIORITY_TIME_CRITICAL);
       break;
     default:
       NOTREACHED() << "Unknown priority.";
       break;
   }
 }
 
 }  // namespace base