Hi Mandy,
On 16/12/2020 12:54 pm, Mandy Chung wrote:
On 12/15/20 5:50 PM, David Holmes wrote:
On 16/12/2020 11:35 am, Serguei Spitsyn wrote:
On Wed, 16 Dec 2020 00:50:04 GMT, David Holmes <dhol...@openjdk.org>
wrote:
The agent class doesn't have to be public it just has to be accessible.
The premain method should be queried for public modifier rather than
just relying on a failed invocation request.
David, thank you for catching this. I'm probably missing something here.
If the agent class is not public then the `m.canAccess(null)` check
is not passed and IAE is thrown with the message:
`Exception in thread "main" java.lang.IllegalAccessException:
method NonPublicAgent.premain must be declared public`
But the `NonPublicAgent.premain` is declared public as below:
public static void premain(String agentArgs, Instrumentation
inst) {
System.out.println("premain: NonPublicAgent was loaded");
}
It seems, the IAE is thrown because the agent class is not public.
Does it mean the `m.canAccess(null)` check is not fully correct?
canAccess will check both the type accessibility and the method
accessibility.
The premain class is not required to be public
(I think you meant "exported").
No I meant "public" in the context of the earlier issue that relaxed
that constraint.
The premain method must be public as this issue about. I expect the
agent module must export the package of the Premain-Class (or
Agent-Class) premain method (qualifiedly or unconditionally) for
java.instrument to access. The spec does not state clearly if the agent
class is public but as it requires the premain method public, it is
sensible to say it's accessible (otherwise, premain method does not have
to be public), i.e. public premain method in a public (accessible)
class). So in the modular world, the public method in a public class
in a package exported at least to java.instrument.
Are you expecting differently?
There are two issues:
1. What does the spec say about the premain class and the premain method
For the method it says it must be public - hence this issue.
For the class it says nothing and we know we've previously relaxed a
check that forced it to be public when not required by the spec.
2. What accessibility will allow the instrumentation code to actually
call the premain method?
In a modular world this likely means the premain class must be exported
somehow. That might mean it is public in an exported package, or perhaps
it may mean that the appropriate --add-opens has been added to the
command-line; or that it has been explicitly exported to allow access
from the instrumentation package. I'm not at all clear which checks in a
modular world can be circumvented by calling setAccessible(true), but
I'm assuming that at least some scenario can be handled by continuing to
use that. If after calling setAccessible(true) we still get an
IllegalAccessException for a public premain method then we can give some
general error message about the premain class needing to be accessible
to the instrumentation package.
Cheers,
David
-----
The spec should clarify. FWIW. I just realized that
https://bugs.openjdk.java.net/browse/JDK-6932391 is not resolved.
so it may not be right to checks its accessibility as that may fail
even though the premain method is public and should be invoked. In
that regard I think the setAccessible call has to remain to deal with
this situation. So the process would be:
- load premain class via appLoader
- lookup premain method using getDeclaredMethod()
- check premain method is public and reject if not
- call setAccessible(true) in case premain class is not accessible
- do invoke
David
Mandy
