'Re: RFR: 8049695: nsk/jdb/options/connect/connect003 fails with "Launched jdb could not attach to de'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8049695: nsk/jdb/options/connect/connect003 fails with "Launched jdb could not attach to de
From:       Chris Plummer <chris.plummer () oracle ! com>
Date:       2018-03-23 20:17:57
Message-ID: b8344620-da01-e304-59a6-f55949060b40 () oracle ! com
[Download RAW message or body]

+1

On 3/22/18 4:50 PM, David Holmes wrote:
> Thanks Alex! Looks good.
>
> David
>
> On 23/03/2018 9:28 AM, Alex Menkov wrote:
>> 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



[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic