On Mon, 24 Oct 2022 05:29:26 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> Various debugger tests (mainly JCK vm/jdwp tests) are failing with: > > `FATAL ERROR in native method: JDWP SetTag, > jvmtiError=JVMTI_ERROR_WRONG_PHASE(112) ` > > Sometimes instead of `SetTag` the message says `GetTag` or `signature`. This > is a new issue caused by > [JDK-8295375](https://bugs.openjdk.org/browse/JDK-8295375), which changed the > debug agent CLASS_PREPARE event handling that is used for class tracking. It > moved it from the main debug agent JVMTIEnv to a JVMTIEnv that already > existed for class tracking purposes to deal with OBJECT_FREE events. Because > of this, synchronizing around VMDEATH was lost, so it's possible for the > CLASS_PREPARE event to be received just before VMDEATH, but then VMDEATH > completes before the CLASS_PREPARE callback can complete. This results in > WRONG_PHASE errors when making some JVMTI calls. > > The easiest fix is for the code in `classTrack_addPreparedClass()` to check > for JVMTI_ERROR_WRONG_PHASE, and if it gets it then just return. It should > also assert that `gData->vmDead` is true when this happens. This is needed in > 3 places where JVMTI is called. > > The purist fix would be to somehow synchronize with VMDEATH, either with the > same locking done by the main event handling code (for example, use > BEGIN_CALLBACK and END_CALLBACK), or by having the class tracking JVMTIEnv > also enable VMDEATH events. The VMDEATH callback block would disable the > CLASS_PREPARE events, and also block until any currently executing > CLASS_PREPARE callback has exited. I think this is overkill, and also adds > locking overhead to the CLASS_PREPARE callback, so I'm going with the simpler > solution of allowing WRONG_PHASE. Thanks for adding is_wrong_phase() with the comments about why it is okay. The deletion of the 'gdata != NULL' check also caught my eye, but I'm getting more and more convinced that it's okay to catch this kind of "can't happen" error via the deref of the pointer that shouldn't be NULL... Thumbs up. ------------- Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.org/jdk/pull/10831
