Diff
Modified: branches/safari-600.1.4.15-branch/Source/_javascript_Core/API/tests/testapi.mm (179899 => 179900)
--- branches/safari-600.1.4.15-branch/Source/_javascript_Core/API/tests/testapi.mm 2015-02-11 00:10:40 UTC (rev 179899)
+++ branches/safari-600.1.4.15-branch/Source/_javascript_Core/API/tests/testapi.mm 2015-02-11 00:12:18 UTC (rev 179900)
@@ -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: branches/safari-600.1.4.15-branch/Source/_javascript_Core/ChangeLog (179899 => 179900)
--- branches/safari-600.1.4.15-branch/Source/_javascript_Core/ChangeLog 2015-02-11 00:10:40 UTC (rev 179899)
+++ branches/safari-600.1.4.15-branch/Source/_javascript_Core/ChangeLog 2015-02-11 00:12:18 UTC (rev 179900)
@@ -1,5 +1,74 @@
2015-02-10 Babak Shafiei <bshaf...@apple.com>
+ Merge r179576, r179648.
+
+ 2015-02-04 Mark Lam <mark....@apple.com>
+
+ r179576 introduce a deadlock potential during GC thread suspension.
+ <https://webkit.org/b/141268>
+
+ Reviewed by Michael Saboff.
+
+ http://trac.webkit.org/r179576 introduced a potential for deadlocking.
+ In the GC thread suspension loop, we currently delete
+ MachineThreads::Thread that we detect to be invalid. This is unsafe
+ because we may have already suspended some threads, and one of those
+ suspended threads may still be holding the C heap lock which we need
+ for deleting the invalid thread.
+
+ The fix is to put the invalid threads in a separate toBeDeleted list,
+ and delete them only after GC has resumed all threads.
+
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::removeCurrentThread):
+ - Undo refactoring removeThreadWithLockAlreadyAcquired() out of
+ removeCurrentThread() since it is no longer needed.
+
+ (JSC::MachineThreads::tryCopyOtherThreadStacks):
+ - Put invalid Threads on a threadsToBeDeleted list, and delete those
+ Threads only after all threads have been resumed.
+
+ (JSC::MachineThreads::removeThreadWithLockAlreadyAcquired): Deleted.
+ * heap/MachineStackMarker.h:
+
+ 2015-02-03 Mark Lam <mark....@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:
+
+2015-02-10 Babak Shafiei <bshaf...@apple.com>
+
Merge r179187.
2015-01-27 Csaba Osztrogonác <o...@webkit.org>
Modified: branches/safari-600.1.4.15-branch/Source/_javascript_Core/heap/MachineStackMarker.cpp (179899 => 179900)
--- branches/safari-600.1.4.15-branch/Source/_javascript_Core/heap/MachineStackMarker.cpp 2015-02-11 00:10:40 UTC (rev 179899)
+++ branches/safari-600.1.4.15-branch/Source/_javascript_Core/heap/MachineStackMarker.cpp 2015-02-11 00:12:18 UTC (rev 179900)
@@ -240,14 +240,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,15 +456,58 @@
MutexLocker lock(m_registeredThreadsMutex);
+ Thread* threadsToBeDeleted = nullptr;
+
#ifndef NDEBUG
// Forbid malloc during the gather phase. The gather phase suspends
// threads, so a malloc during gather would risk a deadlock with a
// thread that had been suspended while holding the malloc lock.
fastMallocForbid();
#endif
- 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;
+ Thread* previousThread = nullptr;
+ 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));
+
+ // Put the invalid thread on the threadsToBeDeleted list.
+ // We can't just delete it here because we have suspended other
+ // threads, and they may still be holding the C heap lock which
+ // we need for deleting the invalid thread. Hence, we need to
+ // defer the deletion till after we have resumed all threads.
+ Thread* nextThread = thread->next;
+ thread->next = threadsToBeDeleted;
+ threadsToBeDeleted = thread;
+
+ if (previousThread)
+ previousThread->next = nextThread;
+ else
+ m_registeredThreads = nextThread;
+ thread = nextThread;
+ continue;
+ }
+#else
+ UNUSED_PARAM(numberOfThreads);
+ ASSERT_UNUSED(success, success);
+#endif
+ }
+ previousThread = thread;
+ thread = thread->next;
}
// It is safe to access the registeredThreads list, because we earlier asserted that locks are being held,
@@ -478,6 +525,11 @@
#ifndef NDEBUG
fastMallocAllow();
#endif
+ for (Thread* thread = threadsToBeDeleted; thread; ) {
+ Thread* nextThread = thread->next;
+ delete thread;
+ thread = nextThread;
+ }
}
}
