[jdk17] RFR: 8271251: JavaThread::java_suspend() fails with "fatal error: Illegal threadstate encountered: 6"

[David Holmes]

On Mon, 26 Jul 2021 18:23:39 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

> Hi,
> 
> Please review this small patch. When self-suspending, a JavaThread might reach SafepointSynchronize::block() with a state of _thread_in_vm which is not listed as a valid state for safepoint polling. There are a couple of simple ways to fix this. As suggested by @dholmes-ora  we can avoid the handshake machinery altogether and directly self-suspend. Since this issue is intermittent and in tier7 which I didn't run as many times as other lower tiers it escaped my testing of 8270085.
> 
> Testing in mach5 tiers1-7. I also reproduced the test failures locally and verified that now both hs202t002.java and ThreadSuspendSelf.java are passing. 
> 
> Thanks,
> Patricio

Looks good! Thanks for fixing this Patricio.

A couple of minor suggestions.

David

src/hotspot/share/runtime/handshake.cpp line 615:

> 613:   assert(_lock.owned_by_self(), "Lock must be held");
> 614:   assert(!_handshakee->has_last_Java_frame() || _handshakee->frame_anchor()->walkable(), "should have walkable stack");
> 615: 

Can we add an assert:

`assert(_handshakee->thread_state() == _thread_blocked, "Caller should have transitioned to _thread_blocked);
`
?

src/hotspot/share/runtime/handshake.cpp line 616:

> 614:   assert(!_handshakee->has_last_Java_frame() || _handshakee->frame_anchor()->walkable(), "should have walkable stack");
> 615: 
> 616:   log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " suspended", p2i(_handshakee));

I think I prefer this logging to remain in the while loop. By placing it there you know the thread will actually wait() and you can also see if there are multiple suspend requests. It might even make sense to put the resume log statement in the loop after the wait() with an explicit check of is_suspended() to preclude a spurious wakeup.

-------------

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk17/pull/283