On Mon, 18 Mar 2024 13:14:41 GMT, Richard Reingruber <rr...@openjdk.org> wrote:

>> This pr changes  `JfrJvmtiAgent::retransform_classes()` and 
>> `jfr_set_enabled` to switch to `WXWrite` before transitioning to the vm.
>> 
>> Testing:
>> make test TEST=jdk/jfr/event/runtime/TestClassLoadEvent.java 
>> TEST_VM_OPTS=-XX:+AssertWXAtThreadSync
>> make test TEST=compiler/intrinsics/klass/CastNullCheckDroppingsTest.java 
>> TEST_VM_OPTS=-XX:+AssertWXAtThreadSync
>> 
>> More tests are pending.
>
> Richard Reingruber 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 three additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into 8327990_jfr_enters_vm_without_wxwrite
>  - Set WXWrite at more locations
>  - Switch to WXWrite before entering the vm

I still feel that this should be fixed inside the `ThreadInVMfromNative` 
transition - the number of callsites that need it just reinforces that for me. 
Granted we then need to look at where we would now have redundant calls.

That said we have had a lot of people looking at the overall WX state 
management logic in the past week or so due to 
https://bugs.openjdk.org/browse/JDK-8327860. The workaround there requires us 
to be in EXEC mode and there are a number of folk who are questioning why "we" 
chose WRITE mode as the default with a switch to EXEC, instead of EXEC as the 
default with a switch to WRITE. But whichever way that goes I think the VM 
state transitions are the places to enforce that choice.

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

PR Review: https://git.openjdk.org/jdk/pull/18238#pullrequestreview-1945158370

Reply via email to