RFR: 8343938: TestStressBailout triggers "Should not be locked when freed" assert
Theo Weidmann
tweidmann at openjdk.org
Tue Jan 21 09:46:07 UTC 2025
This patch addresses a race condition that occurs when the compiler queues shut down because compilation was disabled forever.
The following changes enforce a scheduling such that the race condition (almost) always occurs and the JVM crashes:
diff --git a/src/hotspot/share/compiler/compileBroker.cpp b/src/hotspot/share/compiler/compileBroker.cpp
index 518a53ca26e..61434d1f5f2 100644
--- a/src/hotspot/share/compiler/compileBroker.cpp
+++ b/src/hotspot/share/compiler/compileBroker.cpp
@@ -371,6 +371,10 @@ void CompileQueue::free_all() {
MutexLocker ct_lock(current->lock());
current->lock()->notify();
}
+ do {
+ sleep(2);
+ } while (current->is_free());
+ tty->print_cr("!! other thread has the lock! let's be fast and call free to crash!");
// Put the task back on the freelist.
CompileTask::free(current);
}
@@ -1739,6 +1743,8 @@ void CompileBroker::wait_for_completion(CompileTask* task) {
free_task = true;
while (!task->is_complete() && !is_compilation_disabled_forever()) {
ml.wait();
+ tty->print_cr("I woke up and will give the other thread 10 seconds to crash!");
+ sleep(10);
}
}
The reason for the race condition is that in `CompileQueue::free_all()` the wake up (`notify`, line 372 below) to the thread waiting for the task is immediately succeeded by calling `CompileTask::free` on the task:
https://github.com/openjdk/jdk/blob/4b4b1e912a3193cc95c956acc770015f707449b1/src/hotspot/share/compiler/compileBroker.cpp#L361-L375
`CompileTask::free` asserts that the task is not locked:
https://github.com/openjdk/jdk/blob/4b4b1e912a3193cc95c956acc770015f707449b1/src/hotspot/share/compiler/compileTask.cpp#L65-L69
However, as shown by the diff above, a scheduling exists, where the waiting thread in `CompileBroker::wait_for_completion` will acquire the lock as part of the wake up (line 1741 below) *before* the other thread calls `CompileTask::free` but *not make any progress* (that is, keeps the lock) before the other thread executes `CompileTask::free`. In this case, the assert triggers as the lock is locked.
https://github.com/openjdk/jdk/blob/4b4b1e912a3193cc95c956acc770015f707449b1/src/hotspot/share/compiler/compileBroker.cpp#L1737-L1749
The patch adds a counter to each task to indicate if other threads are waiting for the task. `CompileQueue::free_all` wakes up all waiting threads before continuing and only tries to free the task itself if no other threads were waiting.
Even though JVMCI implements a slightly similar wait/notify structure, JVMCI is beyond the scope of this issue and is not directly affected by the changed code.
-------------
Commit messages:
- Fix var declaration
- Update compileBroker.cpp
- Update compileBroker.cpp
- Fix rc
Changes: https://git.openjdk.org/jdk/pull/23195/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23195&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8343938
Stats: 33 lines in 3 files changed: 29 ins; 3 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/23195.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/23195/head:pull/23195
PR: https://git.openjdk.org/jdk/pull/23195
More information about the hotspot-compiler-dev
mailing list