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.
test/jdk/java/lang/Thread/virtual/TracePinnedThreads.java line 105:
> 103: */
> 104: @Test
> 105: void testContention() throws Exception {
I think this test method expects to have all the launched threads to print to
System.out without hanging. If it hangs then we expect the test run to timeout
with a jtreg timeout. Given the context of this PR in which this test method is
being introduced, I think I can understand what it does. But maybe we should
update the test method's comment to say that this method verifies that
contention when writing to System.out when pinned doesn't cause any hangs?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17221#discussion_r1440252673
