OpenJ9 disallows adding methods in classRedefinition but OpenJDK does not

[tajila]

The jvmti spec states:

"The redefinition may change method bodies, the constant pool and attributes. The redefinition must not add, remove or rename fields or methods, change the signatures of methods, change modifiers, or change inheritance. These restrictions may be lifted in future versions."

However, Oracle's JVM allows one to add new methods. For example:

Redefining C1 to C2 (as seen below) should fail with JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_ADDED according to the spec since C2 adds a new method 'checkUses1'. When this is redefined with Oracle, it does not fail. This is also true for retransformation. OpenJ9 will throw an error when attempting to redefine C1 to C2.

class C1 {
    public static boolean checkReads() {
        return false;
    }

    public static boolean checkUses() {
        return false;
    }

    public static boolean checkProvides() {
        return false;
    }
}

class C2 {
    public static boolean checkReads() {
        return false;
    }

    public static boolean checkUses() {        
        return true;
    }

    public static boolean checkProvides() {
        return true;
    }
    
    private static boolean checkUses1() {
        return false;
    }
}

I have noticed that this behaviour only occurs when 'private static' and 'private final' methods are added/removed. For example if C2 is modified to the following:

class C1 {
    public static boolean checkReads() {
        return false;
    }

    public static boolean checkUses() {
        return false;
    }

    public static boolean checkProvides() {
        return false;
    }
}

class C2 {
    public static boolean checkReads() {
        return false;
    }

    public static boolean checkUses() {        
        return true;
    }

    public static boolean checkProvides() {
        return true;
    }
    
    public static boolean checkUses1() {
        return false;
    } 
}

Then Oracle correctly rejects the redefinition with JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_ADDED.

[tajila]

We have asked Oracle to clarify the spec with regards to this issue. I suspect that they have allowed adding/removing methods to support redefinition of method bodies that may add/remove lambdas as Javac generates extra private static synthetic methods for lambdas.

We want to make sure we have accurately captured all the cases where they have allowed the addition/removal of methods.

[DanHeidinga]

Marking this as a question as the behaviour difference (and required spec update) are something we need to follow up on with the OpenJDK community.

[tajila]

Suman Mitra, from IBM, raised this issue with Oracle. They indicated they were looking into the issue.

[DanHeidinga]

Issue was also raised on a Valhalla-EG call while discussing other JVMTI spec changes:

JVMTI RedefineClasses spec handling of private methods is being tracked via: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8192936&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=LBQnmyrHQkEBElM8bAxhzfwLG2HbsYDdzEznFrQoob4&m=KyGIkr3_m7an4VEDrmlaWzy_eUhQvpypda7FucEbf-M&s=PAZDayMHueNWfP6wI6s3I-XfDpiobFf3OPhEerTcI7s&e=

thanks,
Karen

http://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2018-February/000575.html

OpenJ9 will incur costs to support adding static methods to classes - in the class case it isn't so bad but adding them to interfaces will change the shape of the itable.

With nestmates coming, even private final instance methods will need vtable slots as per the guidance to use invokevirtual not invokespecial for them. This will be an expensive operation.

[tajila]

Oracle has stated that the spec will be relaxed to allow addition and removal of methods. No changes will be made to hotspot.

Oracle has now closed the issue.

[briangoetz]

This is being tracked in OpenJDK as https://bugs.openjdk.java.net/browse/JDK-8192936

[DanHeidinga]

Thanks for the link @briangoetz! We're eagerly following along with the discussion on this topic.

One of the things I've struggled to find is a rationale behind the behaviour change. Any light that can be shed on that, either here or in the OpenJDK bug, would be appreciated.

[briangoetz]

I don't have any additional specifics, but if I had to guess, I would think that someone was making a distinction between externally visible API, which would affect the linkage of instructions from other classes to this one, and members that are part of the implementation (private methods). Specifically, because a lambda must have a lambda body method somewhere, and that method is private, it probably seemed like it was merely an implementation detail, and therefore OK to reap dead lambda body methods and inject new ones. But, of course, the spec doesn't allow that, and either should have been respected or revised.

[DanHeidinga]

Thanks Brian. My initial assumptions were that it was lambda related but everything referred to a Netbeans profiler issue which seemed at first glance unrelated.

[bsideup]

Hi! Any updates here?

It seems that one of our projects does not work on OpenJ9 because of it ( reactor/BlockHound#21 ).

This page explains pretty well why we need it:
https://github.com/reactor/BlockHound/blob/master/docs/how_it_works.md#blocking-jvm-native-method-detection

[DanHeidinga]

Hi @bsideup. Through discussions with Oracle and the OpenJDK community, it's become clear that the ability to add / remove private methods is problematic from a specification perspective and it is very difficult to give add/remove methods a consistent coherent behaviour.

The introduction of NestMates in Java 11 exposed the fact that this isn't a purely local resolution decision anymore as nest members can call each other's private methods.

OpenJDK is actually deprecating this behaviour and plan to remove it in a future release [1].

Intercepting native methods can still be done if the modification is handled in the ClassFileLoadHook rather then through a redefinition/retransformation. Does reactor/BlockHound support late attach or is the early attach + ClassFileLoadHook a sufficient workaround?

[1] https://bugs.openjdk.java.net/browse/JDK-8192936?focusedCommentId=14254266&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14254266

[bsideup]

Hi @DanHeidinga,

BlockHound works as a late attach agent with Java-based instrumentation. We initially implemented it as a normal agent (premain + -javaagent:), but it wasn't easy for the users to add it during the tests (especially in Maven).

I'm super sad to hear that this will not work in future :(
Also, doing the instrumentation in C instead of a very convenient Java ASM API is.... challenging.

[DanHeidinga]

@bsideup have you investigated using the NativeMethodBind jvmti event to replace the natives that BlockHound wants to intercept? It requires more C/C++ or assembly code generation to get the same effect but its likely to remain supported...

[bsideup]

@DanHeidinga yes, our previous implementation was using NativeMethodBind.
We had to do a few hacks to re-bind the methods and do the methodName lookups (IIRC Thread.sleep and similar methods are not lookup-able first time you bind, we had to collect methodIds and symbol offsets and re-bind them). Something like this:
https://gist.github.com/bsideup/556190d424f7b5879e4fb736b627f6d6

[DanHeidinga]

@bsideup That's a really cool use of C++ lambdas!

[bsideup]

I have a slightly less positive word for it, but thanks :D

P.S. ehh..... I was so happy to keep the instrumentation in Java.
I wish we get it solved somehow else when OpenJDK will drop that rule.