Changeset 179753 in webkit
- Timestamp:
- Feb 6, 2015 12:57:45 PM (9 years ago)
- Location:
- trunk/Source/JavaScriptCore
- Files:
-
- 6 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/JavaScriptCore/API/tests/testapi.mm
r179576 r179753 473 473 } 474 474 475 static void* threadMain(void* contextPtr)475 static void* useVMFromOtherThread(void* contextPtr) 476 476 { 477 477 JSContext *context = (__bridge JSContext*)contextPtr; … … 480 480 TestObject *testObject = [TestObject testObject]; 481 481 context[@"testObject"] = testObject; 482 pthread_exit(nullptr); 483 } 484 485 struct ThreadArgs { 486 JSContext* context; 487 volatile bool* mainThreadIsReadyToJoin; 488 volatile bool* otherThreadIsDoneWithJSWork; 489 }; 490 491 static void* useVMFromOtherThreadAndOutliveVM(void* data) 492 { 493 ThreadArgs* args = reinterpret_cast<ThreadArgs*>(data); 494 volatile bool& mainThreadIsReadyToJoin = *args->mainThreadIsReadyToJoin; 495 volatile bool& otherThreadIsDoneWithJSWork = *args->otherThreadIsDoneWithJSWork; 496 497 @autoreleasepool { 498 JSContext *context = args->context; 499 500 // Do something to enter the VM. 501 TestObject *testObject = [TestObject testObject]; 502 context[@"testObject"] = testObject; 503 } 504 otherThreadIsDoneWithJSWork = true; 505 506 while (!mainThreadIsReadyToJoin) 507 usleep(10000); 482 508 pthread_exit(nullptr); 483 509 } … … 1376 1402 1377 1403 pthread_t threadID; 1378 pthread_create(&threadID, NULL, & threadMain, (__bridge void*)context);1404 pthread_create(&threadID, NULL, &useVMFromOtherThread, (__bridge void*)context); 1379 1405 pthread_join(threadID, nullptr); 1380 1406 JSSynchronousGarbageCollectForDebugging([context JSGlobalContextRef]); … … 1382 1408 checkResult(@"Did not crash after entering the VM from another thread", true); 1383 1409 } 1384 1410 1411 @autoreleasepool { 1412 pthread_t threadID; 1413 volatile bool mainThreadIsReadyToJoin = false; 1414 volatile bool otherThreadIsDoneWithJSWork = false; 1415 @autoreleasepool { 1416 JSContext *context = [[JSContext alloc] init]; 1417 ThreadArgs args = { context, &mainThreadIsReadyToJoin, &otherThreadIsDoneWithJSWork }; 1418 1419 pthread_create(&threadID, NULL, &useVMFromOtherThreadAndOutliveVM, &args); 1420 JSSynchronousGarbageCollectForDebugging([context JSGlobalContextRef]); 1421 1422 while (!otherThreadIsDoneWithJSWork) 1423 usleep(10000); 1424 } 1425 1426 mainThreadIsReadyToJoin = true; 1427 pthread_join(threadID, nullptr); 1428 checkResult(@"Did not crash if the VM is destructed before another thread using the VM ends", true); 1429 } 1430 1385 1431 currentThisInsideBlockGetterTest(); 1386 1432 runDateTests(); -
trunk/Source/JavaScriptCore/ChangeLog
r179751 r179753 1 2015-02-06 Mark Lam <mark.lam@apple.com> 2 3 MachineThreads should be ref counted. 4 <https://webkit.org/b/141317> 5 6 Reviewed by Filip Pizlo. 7 8 The VM's MachineThreads registry object is being referenced from other 9 threads as a raw pointer. In a scenario where the VM is destructed on 10 the main thread, there is no guarantee that another thread isn't still 11 holding a reference to the registry and will eventually invoke 12 removeThread() on it on thread exit. Hence, there's a possible use 13 after free scenario here. 14 15 The fix is to make MachineThreads ThreadSafeRefCounted, and have all 16 threads that references keep a RefPtr to it to ensure that it stays 17 alive until the very last thread is done with it. 18 19 * API/tests/testapi.mm: 20 (useVMFromOtherThread): - Renamed to be more descriptive. 21 (useVMFromOtherThreadAndOutliveVM): 22 - Added a test that has another thread which uses the VM outlive the 23 VM to confirm that there is no crash. 24 25 However, I was not actually able to get the VM to crash without this 26 patch because I wasn't always able to the thread destructor to be 27 called. With this patch applied, I did verify with some logging that 28 the MachineThreads registry is only destructed after all threads 29 have removed themselves from it. 30 31 (threadMain): Deleted. 32 33 * heap/Heap.cpp: 34 (JSC::Heap::Heap): 35 (JSC::Heap::~Heap): 36 (JSC::Heap::gatherStackRoots): 37 * heap/Heap.h: 38 (JSC::Heap::machineThreads): 39 * heap/MachineStackMarker.cpp: 40 (JSC::MachineThreads::Thread::Thread): 41 (JSC::MachineThreads::addCurrentThread): 42 (JSC::MachineThreads::removeCurrentThread): 43 * heap/MachineStackMarker.h: 44 1 45 2015-02-06 Commit Queue <commit-queue@webkit.org> 2 46 -
trunk/Source/JavaScriptCore/heap/Heap.cpp
r179728 r179753 312 312 , m_storageSpace(this) 313 313 , m_extraMemoryUsage(0) 314 , m_machineThreads(this)315 314 , m_sharedData(vm) 316 315 , m_slotVisitor(m_sharedData) … … 341 340 #endif 342 341 { 342 m_machineThreads = adoptRef(new MachineThreads(this)); 343 343 m_storageSpace.init(); 344 344 if (Options::verifyHeap()) … … 348 348 Heap::~Heap() 349 349 { 350 // We need to remove the main thread explicitly here because the main thread 351 // may not terminate for a while though the Heap (and VM) is being shut down. 352 m_machineThreads->removeCurrentThread(); 350 353 } 351 354 … … 588 591 GCPHASE(GatherStackRoots); 589 592 m_jitStubRoutines.clearMarks(); 590 m_machineThreads .gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy, registers);593 m_machineThreads->gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy, registers); 591 594 } 592 595 -
trunk/Source/JavaScriptCore/heap/Heap.h
r179728 r179753 120 120 VM* vm() const { return m_vm; } 121 121 MarkedSpace& objectSpace() { return m_objectSpace; } 122 MachineThreads& machineThreads() { return m_machineThreads; }122 MachineThreads& machineThreads() { return *m_machineThreads; } 123 123 124 124 const SlotVisitor& slotVisitor() const { return m_slotVisitor; } … … 356 356 std::unique_ptr<HashSet<MarkedArgumentBuffer*>> m_markListSet; 357 357 358 MachineThreadsm_machineThreads;358 RefPtr<MachineThreads> m_machineThreads; 359 359 360 360 GCThreadSharedData m_sharedData; -
trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp
r179666 r179753 92 92 WTF_MAKE_FAST_ALLOCATED; 93 93 public: 94 Thread( const PlatformThread& platThread, void* base)94 Thread(PassRefPtr<MachineThreads> machineThreads, const PlatformThread& platThread, void* base) 95 95 : platformThread(platThread) 96 96 , stackBase(base) 97 , m_machineThreads(machineThreads) 97 98 { 98 99 #if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN) && defined(SA_RESTART) … … 114 115 PlatformThread platformThread; 115 116 void* stackBase; 117 RefPtr<MachineThreads> m_machineThreads; 116 118 }; 117 119 … … 121 123 #if !ASSERT_DISABLED 122 124 , m_heap(heap) 125 , m_magicNumber(0x1234567890abcdef) 123 126 #endif 124 127 { … … 137 140 t = next; 138 141 } 142 #if !ASSERT_DISABLED 143 m_magicNumber = 0xbaddbeefdeadbeef; 144 #endif 139 145 } 140 146 … … 171 177 172 178 threadSpecificSet(m_threadSpecific, this); 173 Thread* thread = new Thread( getCurrentPlatformThread(), wtfThreadData().stack().origin());179 Thread* thread = new Thread(this, getCurrentPlatformThread(), wtfThreadData().stack().origin()); 174 180 175 181 MutexLocker lock(m_registeredThreadsMutex); … … 181 187 void MachineThreads::removeThread(void* p) 182 188 { 189 ASSERT(static_cast<MachineThreads*>(p)->m_magicNumber == 0x1234567890abcdef); 183 190 static_cast<MachineThreads*>(p)->removeCurrentThread(); 184 191 } … … 188 195 PlatformThread currentPlatformThread = getCurrentPlatformThread(); 189 196 197 // This thread could be the last entity that holds a RefPtr to the 198 // MachineThreads registry. We don't want the registry to be deleted while 199 // we're deleting the the Thread entry, because: 200 // 201 // 1. ~MachineThread() will attempt to lock its m_registeredThreadsMutex 202 // and we already hold it here. m_registeredThreadsMutex is not 203 // re-entrant. 204 // 2. The MutexLocker will unlock m_registeredThreadsMutex at the end of 205 // this function. We can't let it be destructed until before then. 206 // 207 // Using this RefPtr here will defer destructing the registry till after 208 // MutexLocker releases its m_registeredThreadsMutex. 209 RefPtr<MachineThreads> retainMachineThreadsRegistryUntilDoneRemovingThread = this; 210 211 ASSERT(m_magicNumber == 0x1234567890abcdef); 190 212 MutexLocker lock(m_registeredThreadsMutex); 191 213 if (equalThread(currentPlatformThread, m_registeredThreads->platformThread)) { -
trunk/Source/JavaScriptCore/heap/MachineStackMarker.h
r179648 r179753 25 25 #include <setjmp.h> 26 26 #include <wtf/Noncopyable.h> 27 #include <wtf/ThreadSafeRefCounted.h> 27 28 #include <wtf/ThreadSpecific.h> 28 29 #include <wtf/ThreadingPrimitives.h> … … 35 36 class JITStubRoutineSet; 36 37 37 class MachineThreads {38 class MachineThreads : public ThreadSafeRefCounted<MachineThreads> { 38 39 WTF_MAKE_NONCOPYABLE(MachineThreads); 39 40 public: … … 47 48 JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads. 48 49 50 void removeCurrentThread(); 51 49 52 private: 50 53 class Thread; … … 56 59 57 60 static void removeThread(void*); 58 void removeCurrentThread();59 61 60 62 Mutex m_registeredThreadsMutex; … … 63 65 #if !ASSERT_DISABLED 64 66 Heap* m_heap; 67 uint64_t m_magicNumber; // Only used for detecting use after free. 65 68 #endif 66 69 };
Note: See TracChangeset
for help on using the changeset viewer.