RFR: 8316397: StackTrace/Suspended/GetStackTraceSuspendedStressTest.java failed with: SingleStep event is NOT expected [v8]

[Serguei Spitsyn]

On Tue, 8 Apr 2025 23:21:44 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains nine additional commits since the last revision:
>> 
>>  - Merge
>>  - review: add workaround for post_dynamic_code_generated_while_holding_locks
>>  - Merge
>>  - Merge
>>  - review: move suspend point from post_single_step() to get_jvmti_thread_state()
>>  - Merge
>>  - removed obsolete fragment that was not removed in last update
>>  - review: re-fixed the issue as initial fix was wrong
>>  - 8316397: StackTrace/Suspended/GetStackTraceSuspendedStressTest.java failed with: SingleStep event is NOT expected
>
> src/hotspot/share/prims/jvmtiExport.cpp line 428:
> 
>> 426:       // suspend here if there is a suspend request
>> 427:       ThreadBlockInVM tbivm(thread, true /* allow suspend */);
>> 428:     }
> 
> We need some kind of comment before `thread_started` to make it clear why we need to check suspension only on this path. Something like:
> 
> // Within thread_started we could block on a VM mutex which would allow this thread to be marked as suspended,
> // so in some cases we need to honor that suspend request before procedding.

This a reasonable suggestion, thanks! Will add a better comment. In fact, I forgot to add such a comment myself.

> src/hotspot/share/prims/jvmtiExport.hpp line 312:
> 
>> 310:   // is enabled globally then it is created.
>> 311:   // Otherwise, the thread->jvmti_thread_state() is returned.
>> 312:   static JvmtiThreadState* get_jvmti_thread_state(JavaThread *thread, bool allow_suspend = true);
> 
> Please add a detailed comment on what the `allow_suspend` parameter means and when it should be true and false.

Good suggestion, thanks! Will add a comment.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23490#discussion_r2034474656
PR Review Comment: https://git.openjdk.org/jdk/pull/23490#discussion_r2034475376