RFR: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed

[Yasumasa Suenaga]

On Fri, 2 Oct 2020 07:27:43 GMT, Yasumasa Suenaga <ysuenaga at openjdk.org> wrote:

>> I was initially wrong by supporting this, and now I share David's concerns about unclear semantics of this. The
>> questions are:
>>  - Q1: Is it necessary to call the Agent_OnUnload()?
>>  - Q2: Would it be a JVMTI spec violation to call the Agent_OnAttach() multiple times? (It seems to be the case to me.)
>>  - Q3: What has to be done for statically linked agent?
>>  - Q4: Should the agent be correctly loadable in the first place? What were the reasons its loading to fail?
>> 
>> Yes, at least, a CSR is needed for this.
>
>> * 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.

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

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