On Tue, 23 Jan 2024 16:37:35 GMT, Volker Simonis <simo...@openjdk.org> wrote:
>> src/hotspot/share/runtime/init.cpp line 121:
>>
>>> 119: if (AlwaysRecordEvolDependencies) {
>>> 120: JvmtiExport::set_can_hotswap_or_post_breakpoint(true);
>>> 121: JvmtiExport::set_all_dependencies_are_recorded(true);
>>
>> I think the check for AlwaysRecordEvolDependencies needs to be moved into
>> set_can_hotswap_or_post_breakpoint and set_all_dependencies_are_recorded,
>> otherwise don't we risk the value being accidentally reset to false when
>> set_can_hotswap_or_post_breakpoint() is called again by
>> JvmtiManageCapabilities::update()?
>
> A good question, but after deep digging (it took me quite some time to figure
> this out myself :) I don't think that
> `can_hotswap_or_post_breakpoint`/`all_dependencies_are_recorded` can ever be
> reset. Here's how it works:
>
> - there's a global set of `always_capabilities` which is initialized when the
> first JVMTI environment is created:
>
> // capabilities which are always potentially available
> jvmtiCapabilities JvmtiManageCapabilities::always_capabilities;
>
> void JvmtiManageCapabilities::initialize() {
> _capabilities_lock = new Mutex(Mutex::nosafepoint, "Capabilities_lock");
> always_capabilities = init_always_capabilities();
>
>
> - as the name implies, this set of capabilities contains all generally
> available capabilities (except for the `onload` and `solo` capabilites, which
> are maintained in separate sets).
> - `JvmtiManageCapabilities::update()` only operates on this global sets of
> capabilites (and *not* on the concrete capabilities of a specific JVMTI
> environment):
>
> void JvmtiManageCapabilities::update() {
> jvmtiCapabilities avail;
> // all capabilities
> either(&always_capabilities, &always_solo_capabilities, &avail);
> ...
> // If can_redefine_classes is enabled in the onload phase then we know that
> the
> // dependency information recorded by the compiler is complete.
> if ((avail.can_redefine_classes || avail.can_retransform_classes) &&
> JvmtiEnv::get_phase() == JVMTI_PHASE_ONLOAD) {
> JvmtiExport::set_all_dependencies_are_recorded(true);
> }
> ...
> JvmtiExport::set_can_hotswap_or_post_breakpoint(
> avail.can_generate_breakpoint_events ||
> avail.can_redefine_classes ||
> avail.can_retransform_classes);
>
> - This means that `JvmtiManageCapabilities::update()` is always conservative
> regarding its exports to `JvmtiExport` as it always exports *all* potentially
> available capabilites once a JVMTI environment is created and not only the
> specific capabilities requested by concrete JVMTI environment. This means
> that even if we create a JVMTI agent with an empty set of capabilities,
> `can_hotswap_or_post_breakpoint`/`all_dependencies_are_recorded` will still
> be set in `JvmtiExport`.
> - There's no code path (at least I couldn't find one) which takes
> capabilities away from the `always_capabilities` set. Only if an environment
> requests `on_load` capabilities, they will be transferred from the global
> `onload_capabilities` to the `always_capabilities` set:
>
> jvmtiError JvmtiManageCapabilities::add_capabilities(const jvmtiCapabilities
> *current,
> ...
An assert works for me. Thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17509#discussion_r1464120039
