openj9 icon indicating copy to clipboard operation
openj9 copied to clipboard

Consider dropping HCR extensions?

Open jdmpapin opened this issue 1 year ago • 15 comments

That is, allowing the addition and removal of methods and static fields.

I'm curious what the motivation/use case is for these extensions, and whether it might be possible to drop them.


For context, I've been working on a change (#17850) that interacts with class redefinition via MemberName, and the HCR extensions really make matters a lot more complicated when it comes to the invoke and reflect packages. It's not clear what the behaviour is supposed to be, especially when methods are removed.

In trying to clarify the expected behaviour for myself I found that certain interactions are simply broken:

  1. Method instances are not fixed up. They can then call the wrong method or even just segfault.
  2. VirtualHandle (J9 invoke) is not fixed up either, with similar consequences.
  3. DirectHandle (J9 invoke) has its J9Method substituted according to methodPairs, which is only populated for fast HCR. Leaving DirectHandle instances to call the old method bodies is a conceivable choice, but it doesn't look intentional (IMO).
  4. The existing code that tries to fix up MemberName.vmtarget fails to kick in because when it does a lookup into classHashTable the key (originalRAMClass) is uninitialized, so at the moment direct dispatches will unintentionally call the old implementation even with fast HCR.

In my PR, I've fixed (4) - which has to change because I'm changing the meaning of the vmindex field - but only to an extent. The MemberName is fixed up properly whenever the J9JNIMethodID is, which I believe is any time the VM finds a corresponding method (with the same name and signature) in the redefined class. But if the method is removed, then we're out of luck.


Looking for more information online I found https://github.com/eclipse-openj9/openj9/pull/13566#issuecomment-926840082:

The HCR extensions are not well supported and are rarely used, even by debuggers.

And in #771 OpenJDK was found to allow addition/removal of private methods. It was consequently modified to stop doing so.

jdmpapin avatar Jul 25 '23 16:07 jdmpapin