Changeset 179576 in webkit


Ignore:
Timestamp:
Feb 3, 2015, 4:00:00 PM (10 years ago)
Author:
mark.lam@apple.com
Message:

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:
Location:
trunk/Source/JavaScriptCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/API/tests/testapi.mm

    r167326 r179576  
    3030#import "JSExportTests.h"
    3131
     32#import <pthread.h>
     33
    3234extern "C" void JSSynchronousGarbageCollectForDebugging(JSContextRef);
    3335extern "C" void JSSynchronousEdenCollectForDebugging(JSContextRef);
    … …  
    469471    }();
    470472    return containsClass;
     473}
     474
     475static void* threadMain(void* contextPtr)
     476{
     477    JSContext *context = (__bridge JSContext*)contextPtr;
     478
     479    // Do something to enter the VM.
     480    TestObject *testObject = [TestObject testObject];
     481    context[@"testObject"] = testObject;
     482    pthread_exit(nullptr);
    471483}
    472484
    … …  
    13601372    }
    13611373
     1374    @autoreleasepool {
     1375        JSContext *context = [[JSContext alloc] init];
     1376       
     1377        pthread_t threadID;
     1378        pthread_create(&threadID, NULL, &threadMain, (__bridge void*)context);
     1379        pthread_join(threadID, nullptr);
     1380        JSSynchronousGarbageCollectForDebugging([context JSGlobalContextRef]);
     1381
     1382        checkResult(@"Did not crash after entering the VM from another thread", true);
     1383    }
     1384   
    13621385    currentThisInsideBlockGetterTest();
    13631386    runDateTests();

  • trunk/Source/JavaScriptCore/ChangeLog

    r179562 r179576  
     12015-02-03  Mark Lam  <mark.lam@apple.com>
     2
     3        Workaround a thread library bug where thread destructors may not get called.
     4        <https://webkit.org/b/141209>
     5
     6        Reviewed by Michael Saboff.
     7
     8        There's a bug where thread destructors may not get called.  As far as
     9        we know, this only manifests on darwin ports.  We will work around this
     10        by checking at GC time if the platform thread is still valid.  If not,
     11        we'll purge it from the VM's registeredThreads list before proceeding
     12        with thread scanning activity.
     13
     14        Note: it is important that we do this invalid thread detection during
     15        suspension, because the validity (and liveness) of the other thread is
     16        only guaranteed while it is suspended.
     17
     18        * API/tests/testapi.mm:
     19        (threadMain):
     20        - Added a test to enter the VM from another thread before we GC on
     21          the main thread.
     22
     23        * heap/MachineStackMarker.cpp:
     24        (JSC::MachineThreads::removeThreadWithLockAlreadyAcquired):
     25        (JSC::MachineThreads::removeCurrentThread):
     26        - refactored removeThreadWithLockAlreadyAcquired() out from
     27          removeCurrentThread() so that we can also call it for purging invalid
     28          threads.
     29        (JSC::suspendThread):
     30        - Added a return status to tell if the suspension succeeded or not.
     31        (JSC::MachineThreads::tryCopyOtherThreadStacks):
     32        - Check if the suspension failed, and purge the thread if we can't
     33          suspend it.  Failure to suspend implies that the thread has
     34          terminated without calling its destructor.
     35        * heap/MachineStackMarker.h:
     36
    1372015-02-03  Joseph Pecoraro  <pecoraro@apple.com>
    238

  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp

    r179319 r179576  
    191191}
    192192
    193 void MachineThreads::removeCurrentThread()
    194 {
    195     PlatformThread currentPlatformThread = getCurrentPlatformThread();
    196 
    197     MutexLocker lock(m_registeredThreadsMutex);
    198 
    199     if (equalThread(currentPlatformThread, m_registeredThreads->platformThread)) {
     193template<typename PlatformThread>
     194void MachineThreads::removeThreadWithLockAlreadyAcquired(PlatformThread threadToRemove)
     195{
     196    if (equalThread(threadToRemove, m_registeredThreads->platformThread)) {
    200197        Thread* t = m_registeredThreads;
    201198        m_registeredThreads = m_registeredThreads->next;
    … …  
    205202        Thread* t;
    206203        for (t = m_registeredThreads->next; t; t = t->next) {
    207             if (equalThread(t->platformThread, currentPlatformThread)) {
     204            if (equalThread(t->platformThread, threadToRemove)) {
    208205                last->next = t->next;
    209206                break;
    … …  
    215212    }
    216213}
    217 
     214   
     215void MachineThreads::removeCurrentThread()
     216{
     217    PlatformThread currentPlatformThread = getCurrentPlatformThread();
     218
     219    MutexLocker lock(m_registeredThreadsMutex);
     220    removeThreadWithLockAlreadyAcquired(currentPlatformThread);
     221}
     222   
    218223void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
    219224{
    … …  
    227232}
    228233
    229 static inline void suspendThread(const PlatformThread& platformThread)
    230 {
    231 #if OS(DARWIN)
    232     thread_suspend(platformThread);
    233 #elif OS(WINDOWS)
    234     SuspendThread(platformThread);
     234static inline bool suspendThread(const PlatformThread& platformThread)
     235{
     236#if OS(DARWIN)
     237    kern_return_t result = thread_suspend(platformThread);
     238    return result == KERN_SUCCESS;
     239#elif OS(WINDOWS)
     240    bool threadIsSuspended = (SuspendThread(platformThread) != (DWORD)-1);
     241    ASSERT(threadIsSuspended);
     242    return threadIsSuspended;
    235243#elif USE(PTHREADS)
    236244    pthread_kill(platformThread, SigThreadSuspendResume);
     245    return true;
    237246#else
    238247#error Need a way to suspend threads on this platform
    … …  
    453462
    454463    PlatformThread currentPlatformThread = getCurrentPlatformThread();
    455     for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
    456         if (!equalThread(thread->platformThread, currentPlatformThread))
    457             suspendThread(thread->platformThread);
     464    int numberOfThreads = 0; // Using 0 to denote that we haven't counted the number of threads yet.
     465    int index = 1;
     466
     467    for (Thread* thread = m_registeredThreads; thread; index++) {
     468        if (!equalThread(thread->platformThread, currentPlatformThread)) {
     469            bool success = suspendThread(thread->platformThread);
     470#if OS(DARWIN)
     471            if (!success) {
     472                if (!numberOfThreads) {
     473                    for (Thread* countedThread = m_registeredThreads; countedThread; countedThread = countedThread->next)
     474                        numberOfThreads++;
     475                }
     476               
     477                // Re-do the suspension to get the actual failure result for logging.
     478                kern_return_t error = thread_suspend(thread->platformThread);
     479                ASSERT(error != KERN_SUCCESS);
     480
     481                WTFReportError(__FILE__, __LINE__, WTF_PRETTY_FUNCTION,
     482                    "JavaScript garbage collection encountered an invalid thread (err 0x%x): Thread [%d/%d: %p] platformThread %p.",
     483                    error, index, numberOfThreads, thread, reinterpret_cast<void*>(thread->platformThread));
     484               
     485                Thread* nextThread = thread->next;
     486                removeThreadWithLockAlreadyAcquired(thread->platformThread);
     487                thread = nextThread;
     488                continue;
     489            }
     490#else
     491            UNUSED_PARAM(numberOfThreads);
     492            ASSERT_UNUSED(success, success);
     493#endif
     494        }
     495        thread = thread->next;
    458496    }
    459497

  • trunk/Source/JavaScriptCore/heap/MachineStackMarker.h

    r179319 r179576  
    5959        void removeCurrentThread();
    6060
     61        template<typename PlatformThread>
     62        void removeThreadWithLockAlreadyAcquired(PlatformThread);
     63
    6164        Mutex m_registeredThreadsMutex;
    6265        Thread* m_registeredThreads;
Note: See TracChangeset for help on using the changeset viewer.