<!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>[179576] 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/179576">179576</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-02-03 16:00:00 -0800 (Tue, 03 Feb 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Workaround a thread library bug where thread destructors may not get called.
<https://webkit.org/b/141209>
Reviewed by Michael Saboff.
There's a bug where thread destructors may not get called. As far as
we know, this only manifests on darwin ports. We will work around this
by checking at GC time if the platform thread is still valid. If not,
we'll purge it from the VM's registeredThreads list before proceeding
with thread scanning activity.
Note: it is important that we do this invalid thread detection during
suspension, because the validity (and liveness) of the other thread is
only guaranteed while it is suspended.
* API/tests/testapi.mm:
(threadMain):
- Added a test to enter the VM from another thread before we GC on
the main thread.
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::removeThreadWithLockAlreadyAcquired):
(JSC::MachineThreads::removeCurrentThread):
- refactored removeThreadWithLockAlreadyAcquired() out from
removeCurrentThread() so that we can also call it for purging invalid
threads.
(JSC::suspendThread):
- Added a return status to tell if the suspension succeeded or not.
(JSC::MachineThreads::tryCopyOtherThreadStacks):
- Check if the suspension failed, and purge the thread if we can't
suspend it. Failure to suspend implies that the thread has
terminated without calling its destructor.
* heap/MachineStackMarker.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreAPIteststestapimm">trunk/Source/JavaScriptCore/API/tests/testapi.mm</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkercpp">trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkerh">trunk/Source/JavaScriptCore/heap/MachineStackMarker.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreAPIteststestapimm"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/API/tests/testapi.mm (179575 => 179576)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/API/tests/testapi.mm 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/JavaScriptCore/API/tests/testapi.mm 2015-02-04 00:00:00 UTC (rev 179576)
</span><span class="lines">@@ -29,6 +29,8 @@
</span><span class="cx"> #import "DateTests.h"
</span><span class="cx"> #import "JSExportTests.h"
</span><span class="cx">
</span><ins>+#import <pthread.h>
+
</ins><span class="cx"> extern "C" void JSSynchronousGarbageCollectForDebugging(JSContextRef);
</span><span class="cx"> extern "C" void JSSynchronousEdenCollectForDebugging(JSContextRef);
</span><span class="cx">
</span><span class="lines">@@ -470,6 +472,16 @@
</span><span class="cx"> return containsClass;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+static void* threadMain(void* contextPtr)
+{
+ JSContext *context = (__bridge JSContext*)contextPtr;
+
+ // Do something to enter the VM.
+ TestObject *testObject = [TestObject testObject];
+ context[@"testObject"] = testObject;
+ pthread_exit(nullptr);
+}
+
</ins><span class="cx"> void testObjectiveCAPI()
</span><span class="cx"> {
</span><span class="cx"> NSLog(@"Testing Objective-C API");
</span><span class="lines">@@ -1359,6 +1371,17 @@
</span><span class="cx"> checkResult(@"EdenCollection doesn't reclaim new managed values", [managedJSObject value] != nil);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ @autoreleasepool {
+ JSContext *context = [[JSContext alloc] init];
+
+ pthread_t threadID;
+ pthread_create(&threadID, NULL, &threadMain, (__bridge void*)context);
+ pthread_join(threadID, nullptr);
+ JSSynchronousGarbageCollectForDebugging([context JSGlobalContextRef]);
+
+ checkResult(@"Did not crash after entering the VM from another thread", true);
+ }
+
</ins><span class="cx"> currentThisInsideBlockGetterTest();
</span><span class="cx"> runDateTests();
</span><span class="cx"> runJSExportTests();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (179575 => 179576)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-02-04 00:00:00 UTC (rev 179576)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2015-02-03 Mark Lam <mark.lam@apple.com>
+
+ Workaround a thread library bug where thread destructors may not get called.
+ <https://webkit.org/b/141209>
+
+ Reviewed by Michael Saboff.
+
+ There's a bug where thread destructors may not get called. As far as
+ we know, this only manifests on darwin ports. We will work around this
+ by checking at GC time if the platform thread is still valid. If not,
+ we'll purge it from the VM's registeredThreads list before proceeding
+ with thread scanning activity.
+
+ Note: it is important that we do this invalid thread detection during
+ suspension, because the validity (and liveness) of the other thread is
+ only guaranteed while it is suspended.
+
+ * API/tests/testapi.mm:
+ (threadMain):
+ - Added a test to enter the VM from another thread before we GC on
+ the main thread.
+
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::removeThreadWithLockAlreadyAcquired):
+ (JSC::MachineThreads::removeCurrentThread):
+ - refactored removeThreadWithLockAlreadyAcquired() out from
+ removeCurrentThread() so that we can also call it for purging invalid
+ threads.
+ (JSC::suspendThread):
+ - Added a return status to tell if the suspension succeeded or not.
+ (JSC::MachineThreads::tryCopyOtherThreadStacks):
+ - Check if the suspension failed, and purge the thread if we can't
+ suspend it. Failure to suspend implies that the thread has
+ terminated without calling its destructor.
+ * heap/MachineStackMarker.h:
+
</ins><span class="cx"> 2015-02-03 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> Web Inspector: ASSERT mainThreadPthread launching remote debuggable JSContext app with Debug JavaScriptCore
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp (179575 => 179576)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-02-04 00:00:00 UTC (rev 179576)
</span><span class="lines">@@ -190,13 +190,10 @@
</span><span class="cx"> static_cast<MachineThreads*>(p)->removeCurrentThread();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void MachineThreads::removeCurrentThread()
</del><ins>+template<typename PlatformThread>
+void MachineThreads::removeThreadWithLockAlreadyAcquired(PlatformThread threadToRemove)
</ins><span class="cx"> {
</span><del>- PlatformThread currentPlatformThread = getCurrentPlatformThread();
-
- MutexLocker lock(m_registeredThreadsMutex);
-
- if (equalThread(currentPlatformThread, m_registeredThreads->platformThread)) {
</del><ins>+ if (equalThread(threadToRemove, m_registeredThreads->platformThread)) {
</ins><span class="cx"> Thread* t = m_registeredThreads;
</span><span class="cx"> m_registeredThreads = m_registeredThreads->next;
</span><span class="cx"> delete t;
</span><span class="lines">@@ -204,7 +201,7 @@
</span><span class="cx"> Thread* last = m_registeredThreads;
</span><span class="cx"> Thread* t;
</span><span class="cx"> for (t = m_registeredThreads->next; t; t = t->next) {
</span><del>- if (equalThread(t->platformThread, currentPlatformThread)) {
</del><ins>+ if (equalThread(t->platformThread, threadToRemove)) {
</ins><span class="cx"> last->next = t->next;
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="lines">@@ -214,7 +211,15 @@
</span><span class="cx"> delete t;
</span><span class="cx"> }
</span><span class="cx"> }
</span><ins>+
+void MachineThreads::removeCurrentThread()
+{
+ PlatformThread currentPlatformThread = getCurrentPlatformThread();
</ins><span class="cx">
</span><ins>+ MutexLocker lock(m_registeredThreadsMutex);
+ removeThreadWithLockAlreadyAcquired(currentPlatformThread);
+}
+
</ins><span class="cx"> void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
</span><span class="cx"> {
</span><span class="cx"> void* registersBegin = &registers;
</span><span class="lines">@@ -226,14 +231,18 @@
</span><span class="cx"> conservativeRoots.add(stackBegin, stackEnd, jitStubRoutines, codeBlocks);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline void suspendThread(const PlatformThread& platformThread)
</del><ins>+static inline bool suspendThread(const PlatformThread& platformThread)
</ins><span class="cx"> {
</span><span class="cx"> #if OS(DARWIN)
</span><del>- thread_suspend(platformThread);
</del><ins>+ kern_return_t result = thread_suspend(platformThread);
+ return result == KERN_SUCCESS;
</ins><span class="cx"> #elif OS(WINDOWS)
</span><del>- SuspendThread(platformThread);
</del><ins>+ bool threadIsSuspended = (SuspendThread(platformThread) != (DWORD)-1);
+ ASSERT(threadIsSuspended);
+ return threadIsSuspended;
</ins><span class="cx"> #elif USE(PTHREADS)
</span><span class="cx"> pthread_kill(platformThread, SigThreadSuspendResume);
</span><ins>+ return true;
</ins><span class="cx"> #else
</span><span class="cx"> #error Need a way to suspend threads on this platform
</span><span class="cx"> #endif
</span><span class="lines">@@ -452,9 +461,38 @@
</span><span class="cx"> *size = 0;
</span><span class="cx">
</span><span class="cx"> PlatformThread currentPlatformThread = getCurrentPlatformThread();
</span><del>- for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
- if (!equalThread(thread->platformThread, currentPlatformThread))
- suspendThread(thread->platformThread);
</del><ins>+ int numberOfThreads = 0; // Using 0 to denote that we haven't counted the number of threads yet.
+ int index = 1;
+
+ for (Thread* thread = m_registeredThreads; thread; index++) {
+ if (!equalThread(thread->platformThread, currentPlatformThread)) {
+ bool success = suspendThread(thread->platformThread);
+#if OS(DARWIN)
+ if (!success) {
+ if (!numberOfThreads) {
+ for (Thread* countedThread = m_registeredThreads; countedThread; countedThread = countedThread->next)
+ numberOfThreads++;
+ }
+
+ // Re-do the suspension to get the actual failure result for logging.
+ kern_return_t error = thread_suspend(thread->platformThread);
+ ASSERT(error != KERN_SUCCESS);
+
+ WTFReportError(__FILE__, __LINE__, WTF_PRETTY_FUNCTION,
+ "JavaScript garbage collection encountered an invalid thread (err 0x%x): Thread [%d/%d: %p] platformThread %p.",
+ error, index, numberOfThreads, thread, reinterpret_cast<void*>(thread->platformThread));
+
+ Thread* nextThread = thread->next;
+ removeThreadWithLockAlreadyAcquired(thread->platformThread);
+ thread = nextThread;
+ continue;
+ }
+#else
+ UNUSED_PARAM(numberOfThreads);
+ ASSERT_UNUSED(success, success);
+#endif
+ }
+ thread = thread->next;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.h (179575 => 179576)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.h 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.h 2015-02-04 00:00:00 UTC (rev 179576)
</span><span class="lines">@@ -58,6 +58,9 @@
</span><span class="cx"> static void removeThread(void*);
</span><span class="cx"> void removeCurrentThread();
</span><span class="cx">
</span><ins>+ template<typename PlatformThread>
+ void removeThreadWithLockAlreadyAcquired(PlatformThread);
+
</ins><span class="cx"> Mutex m_registeredThreadsMutex;
</span><span class="cx"> Thread* m_registeredThreads;
</span><span class="cx"> WTF::ThreadSpecificKey m_threadSpecific;
</span></span></pre>
</div>
</div>
</body>
</html>