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

[David Holmes]

On 2/09/2020 3:47 pm, Yasumasa Suenaga wrote:
> Hi David,
> 
> On 2020/09/02 14:00, David Holmes wrote:
>> Hi Yasumasa,
>>
>> On 1/09/2020 11:42 pm, Yasumasa Suenaga wrote:
>>> Hi all,
>>>
>>> Please review this change:
>>>
>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8252657
>>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8252657/webrev.00/
>>>
>>> I tried to attach JVMTI agent which returns JNI_ERR in Agent_OnUnload 
>>> via JVMTI.agent_load dcmd. I expected the library would be unloaded, 
>>> but I saw it in VM.dynlibs dcmd even though it was failed to load.
>>
>> I'm not so sure there is a bug here. The specification is quite lax:
>>
>> "Instead the VM ignores the error, or takes some implementation 
>> specific action -- for example it might print an error to standard 
>> error, or record the error in a system log. "
> 
> I think this spec says about VM behavior only. For example, if 
> Agent_OnAttach is failed, the action in VM is not specified.
> 
> 
>> So not unloading the library is not a bug in respect of the 
>> specification. Further by unloading the library do you not now permit 
>> it to be re-loaded and the OnAttach function to be executed 
>> repeatedly? That seems wrong to me. It could also be argued that if 
>> you will re-execute onAttach then you should first have called 
>> OnUnload, which you don't do.
> 
> Administrator can retry to attach agent with another options if it was 
> failed.
> Currently we can attach same agent to one JVM as many times as we like. 
> Of course Agent_OnAttach would be called in each attach operation.

The spec states for startup:

"Exactly one call to a start-up function is made per agent."

and in regards to OnUnload:

" ... and this function is also called if the library is unloaded for 
other reasons."

It says nothing about OnUnload requiring a successful OnAttach for it to 
be called.

If we can attach an agent multiple times and have OnAttach called 
multiple times then that seems a clear violation of the specification. 
At best there would have to be an intervening unloading of the agent 
with a call to OnUnload.

> JVMTI agent might have entry point (e.g. DllMain in Windows) and it 
> might work something (e.g. multiple launch check). In addition it is 
> strange agent library exists even if AgentLibrary is discarded when 
> Agent_OnAttach is failed - nobody would not manage it!

I agree it seems cleaner to unload the library. But I'm not at all clear 
what the expected semantics are and exactly what complies with the 
specification.

David
-----

> 
> 
> Cheers,
> 
> Yasumasa
> 
> 
>> So while this current behaviour may not meet with your expectations it 
>> is not a bug per-se, and the change in behaviour could have other 
>> implications that are not insignificant.
>>
>> If the new behaviour is considered desirable then a CSR request should 
>> also be filed for this.
>>
>> Cheers,
>> David
>> -----
>>
>>> Please see JBS how to reproduce.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa