Title: [292946] trunk/Source/_javascript_Core
- Revision
- 292946
- Author
- [email protected]
- Date
- 2022-04-16 21:22:20 -0700 (Sat, 16 Apr 2022)
Log Message
Fix a deadlock in VMTraps.
https://bugs.webkit.org/show_bug.cgi?id=239421
<rdar://problem/91851592>
Reviewed by Michael Saboff.
The sampling profiler first acquires the codeBlockSet lock followed by the ThreadSuspendLocker.
VMTraps, on the other hand, first acquires the ThreadSuspendLocker followed by the
codeBlockSet lock. As a result, VMTraps can deadlock with the Sampling Profiler
thread, and leave the mutator in a suspended state, or forever blocked on the
codeBlockSet lock.
This was discovered while running the stress/has-indexed-property-with-worsening-array-mode.js.ftl-no-cjit-validate-sampling-profiler
test on a Debug build of jsc on an M1 MBP. Since it requires a race condition to
reproduce, reproducibility is not always reliable.
* runtime/VMTraps.cpp:
(JSC::VMTraps::tryInstallTrapBreakpoints):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (292945 => 292946)
--- trunk/Source/_javascript_Core/ChangeLog 2022-04-17 03:56:33 UTC (rev 292945)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-04-17 04:22:20 UTC (rev 292946)
@@ -1,3 +1,24 @@
+2022-04-16 Mark Lam <[email protected]>
+
+ Fix a deadlock in VMTraps.
+ https://bugs.webkit.org/show_bug.cgi?id=239421
+ <rdar://problem/91851592>
+
+ Reviewed by Michael Saboff.
+
+ The sampling profiler first acquires the codeBlockSet lock followed by the ThreadSuspendLocker.
+ VMTraps, on the other hand, first acquires the ThreadSuspendLocker followed by the
+ codeBlockSet lock. As a result, VMTraps can deadlock with the Sampling Profiler
+ thread, and leave the mutator in a suspended state, or forever blocked on the
+ codeBlockSet lock.
+
+ This was discovered while running the stress/has-indexed-property-with-worsening-array-mode.js.ftl-no-cjit-validate-sampling-profiler
+ test on a Debug build of jsc on an M1 MBP. Since it requires a race condition to
+ reproduce, reproducibility is not always reliable.
+
+ * runtime/VMTraps.cpp:
+ (JSC::VMTraps::tryInstallTrapBreakpoints):
+
2022-04-15 Chris Dumez <[email protected]>
Leverage StringView in more places to avoid some String allocations
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.cpp (292945 => 292946)
--- trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2022-04-17 03:56:33 UTC (rev 292945)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2022-04-17 04:22:20 UTC (rev 292946)
@@ -102,8 +102,17 @@
CallFrame* callFrame = reinterpret_cast<CallFrame*>(context.framePointer);
- Locker codeBlockSetLocker { vm.heap.codeBlockSet().getLock() };
+ // Even though we know the mutator thread is not in C++ code and therefore, not holding
+ // this lock, the sampling profiler may have acquired this lock before acquiring
+ // ThreadSuspendLocker and suspending the mutator. Since VMTraps acquires the
+ // ThreadSuspendLocker first, we can deadlock with the Sampling Profiler thread, and
+ // leave the mutator in a suspended state, or forever blocked on the codeBlockSet lock.
+ Lock& codeBlockSetLock = vm.heap.codeBlockSet().getLock();
+ if (!codeBlockSetLock.tryLock())
+ return;
+ Locker codeBlockSetLocker { AdoptLock, codeBlockSetLock };
+
CodeBlock* foundCodeBlock = nullptr;
EntryFrame* entryFrame = vm.topEntryFrame;
@@ -216,8 +225,17 @@
ASSERT(currentCodeBlock->hasInstalledVMTrapBreakpoints());
VM& vm = currentCodeBlock->vm();
- // We are in JIT code so it's safe to acquire this lock.
- Locker codeBlockSetLocker { vm.heap.codeBlockSet().getLock() };
+ // Even though we know the mutator thread is not in C++ code and therefore, not holding
+ // this lock, the sampling profiler may have acquired this lock before acquiring
+ // ThreadSuspendLocker and suspending the mutator. Since VMTraps acquires the
+ // ThreadSuspendLocker first, we can deadlock with the Sampling Profiler thread, and
+ // leave the mutator in a suspended state, or forever blocked on the codeBlockSet lock.
+ Lock& codeBlockSetLock = vm.heap.codeBlockSet().getLock();
+ if (!codeBlockSetLock.tryLock())
+ return SignalAction::NotHandled;
+
+ Locker codeBlockSetLocker { AdoptLock, codeBlockSetLock };
+
bool sawCurrentCodeBlock = false;
vm.heap.forEachCodeBlockIgnoringJITPlans(codeBlockSetLocker, [&] (CodeBlock* codeBlock) {
// We want to jettison all code blocks that have vm traps breakpoints, otherwise we could hit them later.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
