<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[184229] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/184229">184229</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-05-12 18:47:15 -0700 (Tue, 12 May 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Windows: Cannot use HANDLE from GetCurrentThread() to get the CONTEXT of another thread.
https://bugs.webkit.org/show_bug.cgi?id=144924
Reviewed by Alex Christensen.
The present stack scanning code in the Windows port is expecting that the
GetCurrentThread() API will provide a unique HANDLE for each thread. The code
then saves and later uses that HANDLE with GetThreadContext() to get the
runtime state of the target thread from the GC thread. According to
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683182(v=vs.85).aspx,
GetCurrentThread() does not provide this unique HANDLE that we expect:
"The function cannot be used by one thread to create a handle that can
be used by other threads to refer to the first thread. The handle is
always interpreted as referring to the thread that is using it. A
thread can create a "real" handle to itself that can be used by other
threads, or inherited by other processes, by specifying the pseudo
handle as the source handle in a call to the DuplicateHandle function."
As a result of this, GetCurrentThread() always returns the same HANDLE value, and
we end up never scanning the stacks of other threads because we wrongly think that
they are all equal (in identity) to the scanning thread. This, in turn, results
in crashes due to objects that are incorrectly collected.
The fix is to call DuplicateHandle() to create a HANDLE that we can use. The
MachineThreads::Thread class already accurately tracks the period of time when
we need that HANDLE for the VM. Hence, the life-cycle of the HANDLE can be tied
to the life-cycle of the MachineThreads::Thread object for the corresponding thread.
* heap/MachineStackMarker.cpp:
(JSC::getCurrentPlatformThread):
(JSC::MachineThreads::Thread::Thread):
(JSC::MachineThreads::Thread::~Thread):
(JSC::MachineThreads::Thread::suspend):
(JSC::MachineThreads::Thread::resume):
(JSC::MachineThreads::Thread::getRegisters):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkercpp">trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (184228 => 184229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-05-13 01:47:00 UTC (rev 184228)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-05-13 01:47:15 UTC (rev 184229)
</span><span class="lines">@@ -1,3 +1,42 @@
</span><ins>+2015-05-12 Mark Lam <mark.lam@apple.com>
+
+ Windows: Cannot use HANDLE from GetCurrentThread() to get the CONTEXT of another thread.
+ https://bugs.webkit.org/show_bug.cgi?id=144924
+
+ Reviewed by Alex Christensen.
+
+ The present stack scanning code in the Windows port is expecting that the
+ GetCurrentThread() API will provide a unique HANDLE for each thread. The code
+ then saves and later uses that HANDLE with GetThreadContext() to get the
+ runtime state of the target thread from the GC thread. According to
+ https://msdn.microsoft.com/en-us/library/windows/desktop/ms683182(v=vs.85).aspx,
+ GetCurrentThread() does not provide this unique HANDLE that we expect:
+
+ "The function cannot be used by one thread to create a handle that can
+ be used by other threads to refer to the first thread. The handle is
+ always interpreted as referring to the thread that is using it. A
+ thread can create a "real" handle to itself that can be used by other
+ threads, or inherited by other processes, by specifying the pseudo
+ handle as the source handle in a call to the DuplicateHandle function."
+
+ As a result of this, GetCurrentThread() always returns the same HANDLE value, and
+ we end up never scanning the stacks of other threads because we wrongly think that
+ they are all equal (in identity) to the scanning thread. This, in turn, results
+ in crashes due to objects that are incorrectly collected.
+
+ The fix is to call DuplicateHandle() to create a HANDLE that we can use. The
+ MachineThreads::Thread class already accurately tracks the period of time when
+ we need that HANDLE for the VM. Hence, the life-cycle of the HANDLE can be tied
+ to the life-cycle of the MachineThreads::Thread object for the corresponding thread.
+
+ * heap/MachineStackMarker.cpp:
+ (JSC::getCurrentPlatformThread):
+ (JSC::MachineThreads::Thread::Thread):
+ (JSC::MachineThreads::Thread::~Thread):
+ (JSC::MachineThreads::Thread::suspend):
+ (JSC::MachineThreads::Thread::resume):
+ (JSC::MachineThreads::Thread::getRegisters):
+
</ins><span class="cx"> 2015-05-12 Benjamin Poulain <bpoulain@apple.com>
</span><span class="cx">
</span><span class="cx"> [JSC] Make the NegZero backward propagated flags of ArithMod stricter
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp (184228 => 184229)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-05-13 01:47:00 UTC (rev 184228)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-05-13 01:47:15 UTC (rev 184229)
</span><span class="lines">@@ -72,7 +72,7 @@
</span><span class="cx"> #if OS(DARWIN)
</span><span class="cx"> typedef mach_port_t PlatformThread;
</span><span class="cx"> #elif OS(WINDOWS)
</span><del>-typedef HANDLE PlatformThread;
</del><ins>+typedef DWORD PlatformThread;
</ins><span class="cx"> #elif USE(PTHREADS)
</span><span class="cx"> typedef pthread_t PlatformThread;
</span><span class="cx"> static const int SigThreadSuspendResume = SIGUSR2;
</span><span class="lines">@@ -151,7 +151,7 @@
</span><span class="cx"> #if OS(DARWIN)
</span><span class="cx"> return pthread_mach_thread_np(pthread_self());
</span><span class="cx"> #elif OS(WINDOWS)
</span><del>- return GetCurrentThread();
</del><ins>+ return GetCurrentThreadId();
</ins><span class="cx"> #elif USE(PTHREADS)
</span><span class="cx"> return pthread_self();
</span><span class="cx"> #endif
</span><span class="lines">@@ -176,10 +176,23 @@
</span><span class="cx"> sigemptyset(&mask);
</span><span class="cx"> sigaddset(&mask, SigThreadSuspendResume);
</span><span class="cx"> pthread_sigmask(SIG_UNBLOCK, &mask, 0);
</span><ins>+#elif OS(WINDOWS)
+ ASSERT(platformThread == GetCurrentThreadId());
+ bool isSuccessful =
+ DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(),
+ &platformThreadHandle, 0, FALSE, DUPLICATE_SAME_ACCESS);
+ RELEASE_ASSERT(isSuccessful);
</ins><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> public:
</span><ins>+ ~Thread()
+ {
+#if OS(WINDOWS)
+ CloseHandle(platformThreadHandle);
+#endif
+ }
+
</ins><span class="cx"> static Thread* createForCurrentThread()
</span><span class="cx"> {
</span><span class="cx"> return new Thread(getCurrentPlatformThread(), wtfThreadData().stack().origin());
</span><span class="lines">@@ -228,6 +241,9 @@
</span><span class="cx"> Thread* next;
</span><span class="cx"> PlatformThread platformThread;
</span><span class="cx"> void* stackBase;
</span><ins>+#if OS(WINDOWS)
+ HANDLE platformThreadHandle;
+#endif
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> MachineThreads::MachineThreads(Heap* heap)
</span><span class="lines">@@ -335,7 +351,7 @@
</span><span class="cx"> kern_return_t result = thread_suspend(platformThread);
</span><span class="cx"> return result == KERN_SUCCESS;
</span><span class="cx"> #elif OS(WINDOWS)
</span><del>- bool threadIsSuspended = (SuspendThread(platformThread) != (DWORD)-1);
</del><ins>+ bool threadIsSuspended = (SuspendThread(platformThreadHandle) != (DWORD)-1);
</ins><span class="cx"> ASSERT(threadIsSuspended);
</span><span class="cx"> return threadIsSuspended;
</span><span class="cx"> #elif USE(PTHREADS)
</span><span class="lines">@@ -351,7 +367,7 @@
</span><span class="cx"> #if OS(DARWIN)
</span><span class="cx"> thread_resume(platformThread);
</span><span class="cx"> #elif OS(WINDOWS)
</span><del>- ResumeThread(platformThread);
</del><ins>+ ResumeThread(platformThreadHandle);
</ins><span class="cx"> #elif USE(PTHREADS)
</span><span class="cx"> pthread_kill(platformThread, SigThreadSuspendResume);
</span><span class="cx"> #else
</span><span class="lines">@@ -396,7 +412,7 @@
</span><span class="cx">
</span><span class="cx"> #elif OS(WINDOWS)
</span><span class="cx"> regs.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
</span><del>- GetThreadContext(platformThread, &regs);
</del><ins>+ GetThreadContext(platformThreadHandle, &regs);
</ins><span class="cx"> return sizeof(CONTEXT);
</span><span class="cx"> #elif USE(PTHREADS)
</span><span class="cx"> pthread_attr_init(&regs);
</span></span></pre>
</div>
</div>
</body>
</html>