Protocol version of Attach API

[David Holmes]

On 26/02/2019 10:31 pm, Yasumasa Suenaga wrote:
> Hi Lin,
> 
> I filed this issue to JBS:
>    https://bugs.openjdk.java.net/browse/JDK-8219721

Thanks.

> 
>>       IMHO, It is not easy to distinguish between the case "socket is 
>> blocked for some reason" and "there is no more data from socket", it 
>> is not easy for me to speak under which situation the loop should be 
>> break.
>>      So my idea to fix is using non-blocking socket, do you think it 
>> is reasonable?

Not sure. The first thing read from the socket should be how much data 
is being sent. But I don't know if the protocol can handle that. The 
Solaris door mechanism does seem to allow accurate communication of the 
amount of data, but I haven't had a chance to test this myself.

> 
> I don't think so.
> We can fill NULL values to `buf[]` before end of loop when tail value of 
> buf[] is NULL and `str_count` is less than expected_str_count`.

That doesn't help if the read is blocked.

> I think it is clean if we shutdown socket after the client send request.

?? can we guarantee the receiver will get all the data in that case?

Thanks,
David

> But we have no chance to VirtualMachineImpl or older implementation in 
> earlier JDK releases.
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> On 2019/02/26 20:54, 臧琳 wrote:
>> Dear All,
>>       It seems there is a little complicated for handling the 
>> mentioned issue.
>>       The problem code snippet is 
>> http://hg.openjdk.java.net/jdk/jdk/file/616a32d6b463/src/hotspot/os/linux/attachListener_linux.cpp#l266 
>> to 
>> http://hg.openjdk.java.net/jdk/jdk/file/616a32d6b463/src/hotspot/os/linux/attachListener_linux.cpp#l297. 
>>
>>       The logic is that attachListener keep reading from socket until 
>> the expected_str_count is reached. the expected_str_count is enlarged 
>> with arg_count_max, so it keeps block at reading from socket when 
>> using the old jcmd.
>>       IMHO, It is not easy to distinguish between the case "socket is 
>> blocked for some reason" and "there is no more data from socket", it 
>> is not easy for me to speak under which situation the loop should be 
>> break.
>>      So my idea to fix is using non-blocking socket, do you think it 
>> is reasonable?
>>
>> BRs,
>> Lin
>>
>>
>> ________________________________________
>> From: 臧琳
>> Sent: Tuesday, February 26, 2019 6:19:37 PM
>> To: Yasumasa Suenaga; David Holmes
>> Cc: serviceability-dev at openjdk.java.net 
>> serviceability-dev at openjdk.java.net
>> Subject: Re: Protocol version of Attach API
>>
>> Hi David, Yasumasa, Thomas,
>>      Thanks for point out this issue. it is introduced by enlarged the 
>> arg_count_max for 8215622.
>>      I have reproduced it on my side, and I will handle the fix.
>>      would you like to help create a JBS for this issue?
>>
>>
>> Cheers,
>> Lin
>> ________________________________________
>> From: serviceability-dev <serviceability-dev-bounces at openjdk.java.net> 
>> on behalf of Yasumasa Suenaga <yasuenag at gmail.com>
>> Sent: Tuesday, February 26, 2019 3:25:15 PM
>> To: David Holmes
>> Cc: serviceability-dev at openjdk.java.net 
>> serviceability-dev at openjdk.java.net
>> Subject: Re: Protocol version of Attach API
>>
>> 2019年2月26日(火) 16:11 David Holmes <david.holmes at oracle.com>:
>>>
>>> On 26/02/2019 5:01 pm, Yasumasa Suenaga wrote:
>>>> 2019年2月26日(火) 15:47 Thomas Stüfe <thomas.stuefe at gmail.com>:
>>>>>
>>>>> 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 with Thomas,
>>>> but I think we can update protocol version and reject request(s) from
>>>> jcmd and other tools on earlier release.
>>>
>>> That is not a decision to be taken lightly and one that requires a CSR
>>> request. Simply allowing for an extra arg on the jmap histo subcommand
>>> should not break all backward compatability.
>>>
>>> I think this is a "simple" bug in the Linux (and possibly other) attach
>>> listener logic. It should be reading a protocol "packet" with up to 4
>>> arguments, but it should be perfectly fine to get a packet with fewer
>>> than 4. The current code doesn't do that, but assumes the "packet" is
>>> always the maximum size.
>>
>> Ok,
>> but I can create a patch for Linux only.
>>
>> AFAICS this issue seems to be in AttachListener for Linux, AIX, OS X.
>> (Solaris and Windows are ok)
>>
>> Can you handle this issue?
>> Of course I can take this if it allows for Linux only :-)
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>>> David
>>> -----
>>>
>>>>
>>>> I guess serviceability tools like a jhsdb are recommended in use with
>>>> same version.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>>> ..Thomas
>>>>>