On Mon, 19 Oct 2020 01:32:45 GMT, Yasumasa Suenaga <[email protected]> wrote:

>>> * Q1: Is it necessary to call the Agent_OnUnload()?
>> 
>> [JVMTI spec of 
>> Agent_OnUnload()](https://docs.oracle.com/en/java/javase/15/docs/specs/jvmti.html#onunload)
>>  says this function will be called when the agent library will be unloaded 
>> by platform specific mechanism. OTOH it also says `Agent_OnUnload()` will be 
>> called both at VM termination and **by other reasons**.
>> The spec don't say for the case if `Agent_OnAttach()` would be failed. IMHO 
>> `Agent_OnUnload()` should be called because this PR would unload library if 
>> `Agent_OnAttach()` failed.
>> 
>>> * Q2: Would it be a JVMTI spec violation to call the Agent_OnAttach() 
>>> multiple times? (It seems to be the case to me.)
>> 
>> `Agent_OnAttach()` should be called only once per attach request, but VM 
>> should accept multiple attach request for same agent library.
>> 
>> For example, we can add multiple `-agentlib` and `-agentpath` request as 
>> below. JVMTI agent might change behavior due to arguments or configuration 
>> file.
>> 
>> -agentlib:test=profile=A -agentlib:test=profile=B 
>> -agentpath:/path/to/libtest=profile=C
>> 
>> Agent developers should have responsibility for the behavior when more than 
>> one agent is loaded at a time.
>> 
>>> * Q3: What has to be done for statically linked agent?
>> 
>> JVMTI spec says "unless it is statically linked into the executable", so I 
>> think we can ignore about Agent_OnUnload_L() in this PR.
>> 
>>> * Q4: Should the agent be correctly loadable in the first place? What were 
>>> the reasons its loading to fail?
>> 
>> Agent (`Agent_OnAttach()`) might fail due to error in agent logic. For 
>> example, some agents load configuration file at initialization. If the user 
>> gives wrong value, it will fail.
>> 
>>> Yes, at least, a CSR is needed for this.
>> 
>> I will file CSR for this PR after this discussion.
>
> If we can change the spec that agent library would not be unloaded when 
> `Agent_OnAttach()` failed, we can change like 
> [webrev.00](https://cr.openjdk.java.net/~ysuenaga/JDK-8252657/webrev.00/). It 
> is simple, and similar behavior with `Agent_OnLoad()`. It might be prefer for 
> JVMTI agent developers.

In case of `Agent_OnLoad()`, if it is failed (it returns other than zero), JVM 
is aborted and `Agent_OnUnload()` is not called. I think it is compliant with 
[JVMTI spec of 
Agent_OnUnload()](https://docs.oracle.com/en/java/javase/15/docs/specs/jvmti.html#onunload)
 which says uncontrolled shutdown (aborting JVM) is an exception to this rule.

I will add CSR for this fix, but I want to discuss what we should do before. I 
like that `Agent_OnUnload()` wouldn't be called when `Agent_OnAttach()` is 
failed if we can change the spec - it is consistent and friendly with 
`Agent_OnUnload()`.

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

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

Reply via email to