Protocol version of Attach API

[臧琳]

Dear  Yasumasa,
   The fix you mentioned seems not work as expected.
   I have done an experiment that use jdk1.8's "jcmd <pid> help" to attach to latest jdk.
   it seem the whole "jcmd <pid> help"  buffer is not read completely at one time. in my case it is "jcmd", "<pid>", "help", and then block at while().
   After applied your change, it seems only the "jcmd" is processed, the "<pid>", "help" is replaced by '\0'.

BRs,
Lin
________________________________________
From: serviceability-dev <serviceability-dev-bounces at openjdk.java.net> on behalf of Alex Menkov <alexey.menkov at oracle.com>
Sent: Wednesday, February 27, 2019 6:48:41 AM
To: David Holmes; Chris Plummer; Erik Gahlin; serviceability-dev at openjdk.java.net
Subject: Re: Protocol version of Attach API

On 02/26/2019 13:58, David Holmes wrote:
> On 27/02/2019 7:50 am, Chris Plummer wrote:
>> On 2/26/19 1:30 PM, David Holmes wrote:
>>> On 27/02/2019 5:52 am, Chris Plummer wrote:
>>>> On 2/26/19 9:34 AM, Erik Gahlin wrote:
>>>>> On 2019-02-26 07:47, Thomas Stüfe wrote:
>>>>>> Hi David, Yasumasa,
>>>>>>
>>>>>>
>>>>>>
>>>>>>     > Do we support connection to later VMs from earlier JDK tools?
>>>>>>
>>>>>>     I could not find the spec about this.
>>>>>>     So I asked to serviceability folks before filing this to JBS :-)
>>>>>>
>>>>>>
>>>>>> Just to chime in on that, I do not know if it is specified but it
>>>>>> is certainly very handy in daily use. I often use old jcmd tools
>>>>>> to connect to newer VMs. I always thought that was a neat design.
>>>>>
>>>>> I agree.
>>>>>
>>>>> The tool was designed to be dumb, so it can connect to JVMs
>>>>> regardless of release. If something has changed so that is no
>>>>> longer true, it should be fixed (and backported if needed).
>>>>>
>>>>> Erik
>>>>
>>>> So do we all agree that backporting is acceptable for fixing
>>>> backwards compatibility here? Just want to be clear on that. You
>>>> would need to get the latest 8u to interact with 13 and later, and 7
>>>> and earlier would not work. I'd prefer to keep 8 (and earlier)
>>>> working without any changes to it if possible.
>>>
>>> EEk No! You don't fix this by trying to change all the old versions!
>>> Simply impossible - only the latest updates in those trains would
>>> work. Logistical nightmare as well.
>> That was my thought as well, which is why I called attention to this.
>>>
>>> This is a very simple logic error in the attach logic on Linux (and
>>> possibly elsewhere)! We just need to fix that bug in 13. Yasumasa
>>> already suggested one fix that seems quite simple - it adjusts the
>>> expected amount of data to read if it encounters a logical "EOF". (I
>>> think it can be simplified further but the gist of it seems fine.)
>>>
>>>>
>>>> Also want to point out that this issue might be two-way: 8 can't
>>>> attach to 13 and 13 may have issues attaching to 8 (what happens
>>>> with the extra argument that is sent?).
>>>
>>> You can't expect an old release to accept new and unknown args. That
>>> is perfectly fine.
>>
>> But we should expect that you can attach from 13 to 8. If unknown args
>> are sent, we would need for them to be quietly ignored.
>
> That depends on how well the code was written in 8, but I think it
> perfectly fine to get an error if you try to send a command that 8
> doesn't (fully) understand.

Looks like AttachOperation::arg_count_max is used not just as max arg
count, but also to calculate packet size which needs to be read from the
socket on unix systems (it's expected to be the same for all
operations). So it cannot be easily fixed.
BTW as far as I got Windows if not affected. Looking into the code
(src/hotspot/os/windows/attachListener_windows.cpp) I suppose the
feature implemented by JDK-8215622 does not work at Windows as
Win32AttachListener still support only 3 args.

In my opinion it would be better to revert fix for JDK-8215622 and
rethink how to implement it without changing AttachOperation::arg_count_max

--alex


>
> David
>
>
>>
>> Chris
>>
>>>
>>> David
>>> -----
>>>
>>>> Chris
>>>>
>>>>>
>>>>>>
>>>>>> ..Thomas
>>>>>
>>>>
>>
>>