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

[Alex Menkov]

Hi David,

With too-long shmem name java reports:
ERROR: transport error 202: failed to create shared memory listener: 
Error: address strings longer than 50 characters are invalid
and ret.code is 2

I added checks for both ret.code and presence of "address strings longer 
than" text in the output.
webrev: http://cr.openjdk.java.net/~amenkov/shmem_long_name/webrev_open.06/

--alex

On 03/22/2018 14:32, David Holmes wrote:
> 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