On Wed, 24 Apr 2024 05:44:42 GMT, Chris Plummer <[email protected]> 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 four additional
>> commits since the last revision:
>>
>> - “Merge”
>> - review: updated same clarifying comment in several spots
>> - add comments explaining that the vthread() can return outdated oop
>> - 8330303: Crash: assert(_target_jt == nullptr || _target_jt->vthread() ==
>> target_h()) failed
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 2079:
>
>> 2077: void
>> 2078: GetSingleStackTraceClosure::do_vthread(Handle target_h) {
>> 2079: // Use jvmti_vthread() instead of vthread() as target could have
>> temporary changed
>
> Suggestion:
>
> // Use jvmti_vthread() instead of vthread() as target could have
> temporarily changed
Good catch, fixed now.
> src/hotspot/share/prims/jvmtiEnvBase.hpp line 509:
>
>> 507: void do_vthread(Handle target_h) {
>> 508: assert(_target_jt != nullptr, "sanity check");
>> 509: // Use jvmti_vthread() instead of vthread() as target could have
>> temporary changed
>
> Suggestion:
>
> // Use jvmti_vthread() instead of vthread() as target could have
> temporarily changed
Good catch, thanks. Fixed now.
> src/hotspot/share/prims/jvmtiEnvBase.hpp line 531:
>
>> 529: void do_vthread(Handle target_h) {
>> 530: assert(_target_jt != nullptr, "sanity check");
>> 531: // Use jvmti_vthread() instead of vthread() as target could have
>> temporary changed
>
> Suggestion:
>
> // Use jvmti_vthread() instead of vthread() as target could have
> temporarily changed
Good catch, thanks. Fixed now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18806#discussion_r1577621820
PR Review Comment: https://git.openjdk.org/jdk/pull/18806#discussion_r1577623055
PR Review Comment: https://git.openjdk.org/jdk/pull/18806#discussion_r1577622556