Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1092)

Unified Diff: runtime/vm/os_thread_win.cc

Issue 1978153002: Uses an open thread handle as the ThreadJoinId on Windows. (Closed) Base URL: https://github.com/dart-lang/sdk.git@master
Patch Set: Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: runtime/vm/os_thread_win.cc
diff --git a/runtime/vm/os_thread_win.cc b/runtime/vm/os_thread_win.cc
index e4083dca9c36c55e516fb48b911cc2c00e6224a1..d6bb2429b0306b4e6f9c3fe34265ad38e3dea2d1 100644
--- a/runtime/vm/os_thread_win.cc
+++ b/runtime/vm/os_thread_win.cc
@@ -91,7 +91,7 @@ int OSThread::Start(const char* name,
const ThreadId OSThread::kInvalidThreadId = 0;
-const ThreadJoinId OSThread::kInvalidThreadJoinId = 0;
+const ThreadJoinId OSThread::kInvalidThreadJoinId = NULL;
ThreadLocalKey OSThread::CreateThreadLocal(ThreadDestructor destructor) {
@@ -131,26 +131,15 @@ ThreadId OSThread::GetCurrentThreadTraceId() {
ThreadJoinId OSThread::GetCurrentThreadJoinId() {
- // TODO(zra): Use the thread handle as the join id in order to have a more
- // reliable join on windows.
- return ::GetCurrentThreadId();
+ HANDLE handle = OpenThread(SYNCHRONIZE, false, ::GetCurrentThreadId());
+ ASSERT(handle != NULL);
+ return handle;
}
void OSThread::Join(ThreadJoinId id) {
- HANDLE handle = OpenThread(SYNCHRONIZE, false, id);
-
- // TODO(zra): OSThread::Start() closes the handle to the thread. Thus, by the
- // time we try to join the thread, its resources may have already been
- // reclaimed, and joining will fail. This can be avoided in a couple of ways.
- // First, GetCurrentThreadJoinId could call OpenThread and return a handle.
- // This is bad, because each of those handles would have to be closed.
- // Second OSThread could be refactored to no longer be AllStatic. Then the
- // handle could be cached in the object by the Start method.
- if (handle == NULL) {
- return;
- }
-
+ HANDLE handle = static_cast<HANDLE>(id);
+ ASSERT(handle != NULL);
DWORD res = WaitForSingleObject(handle, INFINITE);
CloseHandle(handle);
ASSERT(res == WAIT_OBJECT_0);
siva 2016/05/16 17:59:27 Are all thread guaranteed to call OSThread::Join()
zra 2016/05/16 20:32:54 Changed to call GetCurrentThreadJoinId only for th

Powered by Google App Engine
This is Rietveld 408576698