Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (179575 => 179576)
--- trunk/Source/_javascript_Core/API/tests/testapi.mm 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm 2015-02-04 00:00:00 UTC (rev 179576)
@@ -29,6 +29,8 @@
#import "DateTests.h"
#import "JSExportTests.h"
+#import <pthread.h>
+
extern "C" void JSSynchronousGarbageCollectForDebugging(JSContextRef);
extern "C" void JSSynchronousEdenCollectForDebugging(JSContextRef);
@@ -470,6 +472,16 @@
return containsClass;
}
+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);
+}
+
void testObjectiveCAPI()
{
NSLog(@"Testing Objective-C API");
@@ -1359,6 +1371,17 @@
checkResult(@"EdenCollection doesn't reclaim new managed values", [managedJSObject value] != nil);
}
+ @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);
+ }
+
currentThisInsideBlockGetterTest();
runDateTests();
runJSExportTests();
Modified: trunk/Source/_javascript_Core/ChangeLog (179575 => 179576)
--- trunk/Source/_javascript_Core/ChangeLog 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-02-04 00:00:00 UTC (rev 179576)
@@ -1,3 +1,39 @@
+2015-02-03 Mark Lam <[email protected]>
+
+ 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:
+
2015-02-03 Joseph Pecoraro <[email protected]>
Web Inspector: ASSERT mainThreadPthread launching remote debuggable JSContext app with Debug _javascript_Core
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (179575 => 179576)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp 2015-02-04 00:00:00 UTC (rev 179576)
@@ -190,13 +190,10 @@
static_cast<MachineThreads*>(p)->removeCurrentThread();
}
-void MachineThreads::removeCurrentThread()
+template<typename PlatformThread>
+void MachineThreads::removeThreadWithLockAlreadyAcquired(PlatformThread threadToRemove)
{
- PlatformThread currentPlatformThread = getCurrentPlatformThread();
-
- MutexLocker lock(m_registeredThreadsMutex);
-
- if (equalThread(currentPlatformThread, m_registeredThreads->platformThread)) {
+ if (equalThread(threadToRemove, m_registeredThreads->platformThread)) {
Thread* t = m_registeredThreads;
m_registeredThreads = m_registeredThreads->next;
delete t;
@@ -204,7 +201,7 @@
Thread* last = m_registeredThreads;
Thread* t;
for (t = m_registeredThreads->next; t; t = t->next) {
- if (equalThread(t->platformThread, currentPlatformThread)) {
+ if (equalThread(t->platformThread, threadToRemove)) {
last->next = t->next;
break;
}
@@ -214,7 +211,15 @@
delete t;
}
}
+
+void MachineThreads::removeCurrentThread()
+{
+ PlatformThread currentPlatformThread = getCurrentPlatformThread();
+ MutexLocker lock(m_registeredThreadsMutex);
+ removeThreadWithLockAlreadyAcquired(currentPlatformThread);
+}
+
void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
{
void* registersBegin = ®isters;
@@ -226,14 +231,18 @@
conservativeRoots.add(stackBegin, stackEnd, jitStubRoutines, codeBlocks);
}
-static inline void suspendThread(const PlatformThread& platformThread)
+static inline bool suspendThread(const PlatformThread& platformThread)
{
#if OS(DARWIN)
- thread_suspend(platformThread);
+ kern_return_t result = thread_suspend(platformThread);
+ return result == KERN_SUCCESS;
#elif OS(WINDOWS)
- SuspendThread(platformThread);
+ bool threadIsSuspended = (SuspendThread(platformThread) != (DWORD)-1);
+ ASSERT(threadIsSuspended);
+ return threadIsSuspended;
#elif USE(PTHREADS)
pthread_kill(platformThread, SigThreadSuspendResume);
+ return true;
#else
#error Need a way to suspend threads on this platform
#endif
@@ -452,9 +461,38 @@
*size = 0;
PlatformThread currentPlatformThread = getCurrentPlatformThread();
- for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
- if (!equalThread(thread->platformThread, currentPlatformThread))
- suspendThread(thread->platformThread);
+ 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;
}
for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.h (179575 => 179576)
--- trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2015-02-03 23:45:31 UTC (rev 179575)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.h 2015-02-04 00:00:00 UTC (rev 179576)
@@ -58,6 +58,9 @@
static void removeThread(void*);
void removeCurrentThread();
+ template<typename PlatformThread>
+ void removeThreadWithLockAlreadyAcquired(PlatformThread);
+
Mutex m_registeredThreadsMutex;
Thread* m_registeredThreads;
WTF::ThreadSpecificKey m_threadSpecific;
