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

[David Holmes]

On 23/03/2018 6:43 AM, Alex Menkov wrote:
> 
> 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.

This doesn't ensure it failed gracefully:

81         // extra test: ensure using of too-long name fails gracefully
82         // (shmemName + "X") is expected to be "too long".
83         ProcessTools.executeProcess(getTarget(shmemName + "X"))
84                 .shouldNotHaveExitValue(0);

It may have crashed. What exactly is the failure mode? return code 1? 
Exception message that we can check for in outputAnalyzer ?

David

> --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