Fix deadlock related to safepoint sync. #279
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`gc_lock` now never does safepoint check. We always explicitly enter safepoint via `ThreadBlockInVM`. The VM companion thread now lets `VM_MMTkSTWOperation` (executed by the VM thread) transition the state to `_threads_resumed` instead of doing it after `VMThread::execute` returns. This works around a problem where `VMThread::execute` continues to block the companion thread when the VM thread starts executing other VM operations. Some minor style changes: We consistently use the constant `Mutex::_no_safepoint_check_flag` instead of `true` when acquiring or waiting for the `Monitor`. Slightly updated comments and logs. Logging statements no longer include thread IDs because thread IDs can be turned on by a command line option.
lusearch used to cause deadlock, and this PR should fix it.
|
Why was this never a problem when we had the coordinator thread? Also it'd be good to get @tianleq to take a look at this PR since he's spent a lot of time understanding the internal safepoint thread mechanism in HotSpot. |
When we had the coordinator thread, After removing the coordinator thread, we removed the mutex and condvar in However, the synchronization problem with OpenJDK's VMThread blocked the "companion thread", which in turn blocked the worker executing So the reason why this problem only manifests after we removed the coordinator thread is that our synchronization mechanism in mmtk-core changed. This problem is very likely to reproduce when executing One reason why In comparison, in DaCapo 2006, it doesn't execute any I think the |
|
I added @wenyuzhao as the other reviewer, and removed myself from the list. |
tianleq
reviewed
May 29, 2024
wenyuzhao
approved these changes
May 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
gc_locknow never does safepoint check. We always explicitly enter safepoint viaThreadBlockInVM.The VM companion thread now lets
VM_MMTkSTWOperation(executed by the VM thread) transition the state to_threads_resumedinstead of doing it afterVMThread::executereturns. This works around a problem where the VM thread fails to unblock the companion thread fromVMThread::executeafter finishing evaluatingVM_MMTkSTWOperation.Also added the
lusearchbenchmark from DaCapo 2006 to normal tests. This test case is known to cause deadlock before, and this PR should fix it.Some minor style changes:
We consistently use the constant
Mutex::_no_safepoint_check_flaginstead oftruewhen acquiring or waiting for theMonitor.Slightly updated comments and logs. Logging statements no longer include thread IDs because thread IDs can be turned on by a command line option.
Fixes: #278