RFR: 8049695: nsk/jdb/options/connect/connect003 fails with "Launched jdb could not attach to debuggee during 300000 milliseconds"

[Alex Menkov]

Updated webrev:
http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open.05/

The test was updated to ensure shmem name longer than 49 symbols causes 
java failure.

--alex


On 03/21/2018 15:39, David Holmes wrote:
> On 22/03/2018 2:41 AM, Alex Menkov wrote:
>> Hi David,
>>
>> On 03/20/2018 21:51, David Holmes wrote:
>>> Hi Alex,
>>>
>>> On 21/03/2018 3:25 AM, Alex Menkov wrote:
>>>> Hi David,
>>>>
>>>> On 03/19/2018 18:10, David Holmes wrote:
>>>>> Hi Alex,
>>>>>
>>>>> On 20/03/2018 10:28 AM, Alex Menkov wrote:
>>>>>> Hi guys,
>>>>>>
>>>>>> please re-review the fix.
>>>>>
>>>>> I still have an unanswered question about where the max of 49 is 
>>>>> enforced. I see it for the "address" but not names in general. ??
>>>>
>>>> for shmem the "channel name" is the address (it's checked in 
>>>> createTransport/openTransport).
>>>> Names for mutexes/events are generated by appending some strings to 
>>>> the adddress and length of the added parts are supposed to be less 
>>>> than MAX_IPC_SUFFIX (25 symbols):
>>>> ".mutex" (+ up to 3 symbols)
>>>> ".hasData" (+ up to 3 symbols)
>>>> ".hasSpace" (+ up to 3 symbols)
>>>> ".ctos"
>>>> ".stoc"
>>>> ".accept" (+ up to 3 symbols)
>>>> ".attach" (+ up to 3 symbols)
>>>> ".<pid>" (pid is a DWORD)
>>>
>>> Okay so ... the code in shmemBase.c is very unclear as to which 
>>> "names" can come in from an external source and which are only ever 
>>> derived from other "names". If the "address" (which seems a very bad 
>>> description in this case!) is the only external source for a name, 
>>> and it is limited to a length of 49 then that is okay.
>>
>> Yes, the "address" is the only external arg, all other names are 
>> constructed from it.
>> I believe it's "address" because it comes from "address" parameter:
>> -Xrunjdwp:transport=st_shmem,address=<shmem_name>
>>
>>>
>>>>>
>>>>>> Reg.test is added the the issue.
>>>>>
>>>>> I don't quite follow the test. I see you try to set the name with a 
>>>>> value that is too long, and if that doesn't cause an overflow and 
>>>>> we don't crash that is good. But I'd expect you to read back the 
>>>>> name and check it matches the truncated name with 49 characters.
>>>>
>>>> The test specifies the maximum length supported (49 symbols)
>>>> (if longer name is specified, "address strings longer than 50 
>>>> characters are invalid" error reported).
>>>
>>> I missed the substring that simply causes the name to be the maximum 
>>> supported length. That would trigger the overflow and so suffices as 
>>> a regression test for this fix.
>>>
>>> Is there another test that already passes a too-long name and 
>>> verifies the error gets thrown?
>>
>> Do you mean name >= 50 symbols?
>> No, there is no such test.
>> I don't think it make much sense (test an arbitrary 
>> implementation-specific restriction), but I can add the case to the test.
> 
> It ensures that using a too-long name fails gracefully.
> 
> Thanks,
> David
> 
>> --alex
>>
>>>
>>>> As far as I see there is no way to read back the name used to create 
>>>> the transport.
>>>
>>> Ok.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>> --alex
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> webrev: 
>>>>>> http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open.04/
>>>>>>
>>>>>> --alex
>>>>>>
>>>>>> On 03/13/2018 16:14, Alex Menkov wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Please review a small fix for
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8049695
>>>>>>> webrev: 
>>>>>>> http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open/
>>>>>>>
>>>>>>> Root cause of the issue is jbd hungs as a result of the buffer 
>>>>>>> overflow.
>>>>>>>
>>>>>>> In the beginning of the shmemBase.c:
>>>>>>>
>>>>>>> #define MAX_IPC_PREFIX 50   /* user-specified or generated name 
>>>>>>> for */
>>>>>>>                              /* shared memory seg and prefix for 
>>>>>>> other IPC */
>>>>>>> #define MAX_IPC_SUFFIX 25   /* suffix to shmem name for other IPC 
>>>>>>> names */
>>>>>>> #define MAX_IPC_NAME   (MAX_IPC_PREFIX + MAX_IPC_SUFFIX)
>>>>>>>
>>>>>>> buffer (char prefix[]) in function createStream is used to 
>>>>>>> generate base name for mutex/events, so MAX_IPC_PREFIX is not big 
>>>>>>> enough.
>>>>>>>
>>>>>>> --alex