On Thu, 27 Oct 2022 18:06:38 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> This PR renamed EI_GC_FINISH to EI_CLASS_UNLOAD and does some other minor
> cleanups related to EI_GC_FINISH.
>
> The debug agent deals with 3 types of events: JVMTI (per the spec), JDWP (per
> the spec), and an event indexing that the debug agent refers to as EI (Event
> Index) events. We have the following EI_GC_FINISH event. The indexing is into
> the array of jdwp event handlers which are created when the debugger sends an
> EventRequest command.
>
> Note there is no EI_CLASS_UNLOAD event that maps to the JDWP CLASS_UNLOAD
> event, but instead we have:
>
> ` EI_GC_FINISH = 8,`
>
> And these are the mappings between EI_GC_FINISH and JDWP and JVMTI events
>
>
> case JVMTI_EVENT_GARBAGE_COLLECTION_FINISH:
> return EI_GC_FINISH;
>
> case JDWP_EVENT(CLASS_UNLOAD):
> return EI_GC_FINISH;
>
> index2jvmti[EI_GC_FINISH - EI_min] = JVMTI_EVENT_GARBAGE_COLLECTION_FINISH;
>
> index2jdwp[EI_GC_FINISH - EI_min] = JDWP_EVENT(CLASS_UNLOAD);
>
>
> So there is this odd relationship between EI_GC_FINISH and the JDWP
> CLASS_UNLOAD event. Note that JVMTI does not have a CLASS_UNLOAD (except for
> unused support in the extension mechanism), and JDWP has no GC_FINISH event.
>
> This relationship between EI_GC_FINISH and the JDWP CLASS_UNLOAD events stems
> from the fact that at one point JVMTI_EVENT_GARBAGE_COLLECTION_FINISH was
> used to trigger the synthesizing all of JDWP CLASS_UNLOAD events for classes
> that unloaded during the last GC. That is no longer the case, and instead
> each time a JVMTI OBJECT_FREE event is triggered for a Class instance, the
> JDWP CLASS_UNLOAD is generated.
>
> Since JDWP CLASS_UNLOAD maps to EI_GC_FINISH, we have the following:
>
> ` node = getHandlerChain(EI_GC_FINISH)->first;`
>
> By looking at this line of code, you would never guess that this is how you
> fetch the event handler chain for JDWP CLASS_UNLOAD EventRequests, but it is.
>
> To clean this up I renamed EI_GC_FINISH to EI_CLASS_UNLOAD. However, that
> still leaves the EI_GC_FINISH to JVMTI_EVENT_GARBAGE_COLLECTION_FINISH
> mapping to deal with. It's not needed anymore. When we get a
> JVMTI_EVENT_GARBAGE_COLLECTION_FINISH, this is all we ever do with it:
>
>
> static void JNICALL
> cbGarbageCollectionFinish(jvmtiEnv *jvmti_env)
> {
> LOG_CB(("cbGarbageCollectionFinish"));
> ++garbageCollected;
> LOG_MISC(("END cbGarbageCollectionFinish"));
> }
>
>
> So the event is not passed around at all, and doesn't trigger other events or
> mapping to a JDWP event. It is never used as an "event index". This means
> jvmti2EventIndex should assert if it is ever passed in, since following
> mapping should never be needed:
>
>
> case JVMTI_EVENT_GARBAGE_COLLECTION_FINISH:
> return EI_GC_FINISH;
>
>
> This is accomplished by simply deleting the above code, and failing through
> to the default error handling code.
>
> Also no longer needed is the following entry:
>
> index2jvmti[EI_GC_FINISH -EI_min] =
> JVMTI_EVENT_GARBAGE_COLLECTION_FINISH
>
> For this we just assign the entry to 0, which will result in an error if the
> entry is ever referenced.
Changes requested by amenkov (Reviewer).
src/jdk.jdwp.agent/share/native/libjdwp/eventHandler.c line 1512:
> 1510: * so it cannot be setup using threadControl_setEventMode(). Use
> JVMTI API directly.
> 1511: */
> 1512: error = JVMTI_FUNC_PTR(gdata->jvmti,SetEventNotificationMode)
Please add space after the comma:
error = JVMTI_FUNC_PTR(gdata->jvmti, SetEventNotificationMode)
src/jdk.jdwp.agent/share/native/libjdwp/util.c line 1983:
> 1981: {
> 1982: jdwpEvent event = 0;
> 1983: if (ei >= EI_min || ei >= EI_max) {
Should be "(ei >= EI_min && ei <= EI_max"
src/jdk.jdwp.agent/share/native/libjdwp/util.c line 1996:
> 1994: {
> 1995: jvmtiEvent event = 0;
> 1996: if (ei >= EI_min || ei >= EI_max) {
Should be "(ei >= EI_min && ei <= EI_max"
-------------
PR: https://git.openjdk.org/jdk/pull/10887
