RFR: 8165276: Spec states to invoke the premain method in an agent class if it's public but implementation differs

[Mandy Chung]

On Wed, 9 Dec 2020 06:40:25 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java line 501:
>> 
>>> 499:             String msg = "method " + classname + "." +  methodname + " must be declared public";
>>> 500:             throw new IllegalAccessException(msg);
>>> 501:         }
>> 
>> If canAccess fails then it means that javaAgentClass is not public or the premain method is not public. The invoke will fail but I agree eat explicit canAccess check means we get finer control on the exception message.
>> 
>> (I can't help feeling that we should do a bit more cleanup here and not use getDeclaredMethod or be concerned with inheritance. This is because the Premain-Class is meant to specify the agent class. Also can't have a public static premain in super class and a non-public static premain in a sub-class).
>
> My gut feeling is that it should be possible to get rid of the getDeclaredMethod calls with the reasoning you provided above. The canAccess check is not really needed in such a case as the getMethod returns public methods only (is my understanding correct?). This would simplify the implementation. Two disadvantages of this approach are:
> - less fine control on the exception message: NoSuchMethodException instead of IllegalAccessException for non-public premain methods
> - some compatibility risk is involved (most of agents define premain method correctly, so the only java agents that define premain methods with unusual tricks are impacted)

If the java agent class declares a non-public premain method but  its superclass declares a public premain method, what should be the expected behavior?   The proposed fix will choose the java agent class's non-public premain and therefore it will fail to load the agent class with IAE.

If we get rid of the `getDeclaredMethod` call and find premain using `getMethod`, it will choose the public premain method defined directly in the java agent class or indirectly in its superclass.

As Alan stated, `Premain-Class` attribute is meant to specify the agent class.  Also specified in the package spec of `java.lang.instrument', the agent class must implement a public static premain method similar in principle to the main application entry point.  Also under "Manifest Attributes" section,
   Premain-Class
        When an agent is specified at JVM launch time this attribute specifies the agent class. That is, the class containing the premain method. 
 It expects that there is a public premain method declared in the agent class, not in its superclass.

So I think it's best to fix this as well in this PR by dropping line 469-495 (i.e. keep `getDeclaredMethod` case and not search for inherited premain).   Throw if the premain method is not found or it is not accessible.   This needs to update the CSR for behavioral change.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1694