On Tue, 2 Jan 2024 13:53:39 GMT, Alan Bateman <al...@openjdk.org> wrote:
> -Djdk.tracePinnedThreads is a debugging option that dates from early > development in the loom repo to identify pinned threads. It has several > issues and this tracing option will eventually be removed (use the JFR events > instead). Several hangs have been reported when running with the system > property set. The "hangs" stem from the onPinned callback executing while the > virtual thread is in a transition state (typically parking). If the virtual > parks while printing the stack trace then it works like a nested park where > the thread state is never restored. Contention on the System.out can also > lead to deadlock when there are platform and pinned virtual threads printing > to System.out around the same time. > > This PR brings over the changes from the loom repo to avoid these hangs. The > changes mean the stack trace is only printed to System.out when the > PrintStream lock can be acquired without blocking. It also restores the > thread state after printing. An alternative to not printing traces would of > course be to queue the traces so they are printed by another thread but this > is just adding complexity for a debugging option that we want to go away. Overall this change looks OK to me. I'm not too familiar with the (virtual) thread states so I can't say if there's any other implication of changing the virtual thread state to RUNNING when we are printing this thread dump. I've a trivial review comment about the new test method introduced in this PR, which I have added inline. ------------- Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17221#pullrequestreview-1801666856
