RFR(S): URG! JDK-8049226 com/sun/jdi/OptionTest.java test times out again

[serguei.spitsyn at oracle.com]

Dmitry,

This looks good in general.
But what error code will be returned in the case of 
AGENT_ERROR_TRANSPORT_INIT ?
Is it Ok to return whatever exit_code we already have or we better 
return some special error code?

Probably, it is Ok to keep the comment at the line 1315 as it is.


Thanks,
Serguei

On 8/26/14 7:47 AM, Dmitry Samersoff wrote:
> Staffan,
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.03/
>
> After couple of experiments I end up with simple fix - don't coredump on
> AGENT_ERROR_TRANSPORT_INIT
>
> Current code don't propagate transport error to upper level, so if we
> need fine grained control I'll rewrite transport initialization.
>
> -Dmitry
>
>
> On 2014-08-25 15:53, Staffan Larsen wrote:
>> On 25 aug 2014, at 13:33, Dmitry Samersoff <dmitry.samersoff at oracle.com
>> <mailto:dmitry.samersoff at oracle.com>> wrote:
>>
>>> Staffan,
>>>
>>> On 2014-08-25 15:26, Staffan Larsen wrote:
>>>> Dmitry,
>>>>
>>>> One problem with this fix is that debugInit_exit() is used from many
>>>> places in the JDWP code. For some of these I think it still does make
>>>> sense to receive a core dump for analysis. I agree that when parsing
>>>> options, we do not need a core dump.
>>> jdwp has explicit option to request a coredump at debugInit_exit().
>>>
>>> Is it enough or I should create a more sophisticated fix ?
>> I don’t think that is enough. Often a problem will happen and will not
>> be reproducible.
>>
>> Maybe this code:
>>
>>      if ((arg.error != JDWP_ERROR(NONE)) &&
>>          (arg.startCount == 0) &&
>>          initOnStartup) {
>>          EXIT_ERROR(map2jvmtiError(arg.error), "No transports initialized");
>>      }
>>
>> can use some variant of EXIT_ERROR that does not call fatalError() ?
>>
>> /Staffan
>>
>>> Actually, I don't think that coredump on every call to jni_FatalError()
>>> (see jni.cpp:726) is necessary but this hotspot code is here for 6 years
>>> and changing it probably out of scope of this fix.
>>>
>>> -Dmitry
>>>
>>>
>>>> /Staffan
>>>>
>>>>
>>>> On 20 aug 2014, at 17:55, Dmitry Samersoff
>>>> <dmitry.samersoff at oracle.com <mailto:dmitry.samersoff at oracle.com>> wrote:
>>>>
>>>>> Serguei,
>>>>>
>>>>> After some additional testing I changed the fix a bit. Please take
>>>>> a look at new version.
>>>>>
>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.02/
>>>>>
>>>>> New version reports JVMTI error to stderr.
>>>>>
>>>>> Also jniFatalError is not referenced any more so I remove it.
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>>
>>>>> On 2014-08-20 12:08, serguei.spitsyn at oracle.com wrote:
>>>>>> Ok.
>>>>>>
>>>>>> Thank you for the explanation! Serguei
>>>>>>
>>>>>> On 8/20/14 1:01 AM, Dmitry Samersoff wrote:
>>>>>>> Serguei,
>>>>>>>
>>>>>>> 1. Historically JDI test-suite had no tests for failed
>>>>>>> transport initialization behavior and invalid parameters
>>>>>>> handling.
>>>>>>>
>>>>>>> 2. As a part of JDWP hardening work I added couple of such
>>>>>>> tests to OptionTest.java - these tests pass invalid parameters
>>>>>>> to dt_socket transport to make sure that transport doesn't
>>>>>>> crash (one such crash was discovered and fixed) but just return
>>>>>>> non-zero exit code to upper level.
>>>>>>>
>>>>>>> 3. After fix for JDK-6694099 any non-zero exit code from
>>>>>>> transport cause VM to coredump. Dumping multiple cores on busy
>>>>>>> machine takes a time so harness kills the test by timeout.
>>>>>>>
>>>>>>> We can just increase timeout for this test but I don't think
>>>>>>> it's a good idea to dump core when invalid parameters passed to
>>>>>>> transport.
>>>>>>>
>>>>>>> So there is the fix.
>>>>>>>
>>>>>>> 4. After the fix tests for negative parameters will return
>>>>>>> non-zero exit code as expected but will not dump the core.
>>>>>>>
>>>>>>> -Dmitry
>>>>>>>
>>>>>>> On 2014-08-20 00:54, serguei.spitsyn at oracle.com wrote:
>>>>>>>> Hi Dmitry,
>>>>>>>>
>>>>>>>> The fix seems to be Ok. Just want to make it clear... This
>>>>>>>> fix just changes the bug pattern. It a case of incorrect
>>>>>>>> transport parameters the test is still going to fail but
>>>>>>>> without crash, right?
>>>>>>>>
>>>>>>>> Thanks, Serguei
>>>>>>>>
>>>>>>>> On 8/19/14 12:09 PM, Dmitry Samersoff wrote:
>>>>>>>>> Hi Everybody,
>>>>>>>>>
>>>>>>>>> Please review the fix:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8049226/webrev.01/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>> JDWP call jniFatalError if transport can't be initialized (e.g. wrong
>>>>>>>>> parameters) and jniFatalError call os::abort().  Therefor
>>>>>>>>> all transport initialization errors cause vm to coredump.
>>>>>>>>>
>>>>>>>>> I see no reason for debugInit_exit to call jniFatalError so
>>>>>>>>> remove this code.
>>>>>>>>>
>>>>>>>>> -Dmitry
>>>>>>>>>
>>>>>
>>>>> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg,
>>>>> Russia * I would love to change the world, but they won't give me
>>>>> the sources.
>>>
>>> -- 
>>> Dmitry Samersoff
>>> Oracle Java development team, Saint Petersburg, Russia
>>> * I would love to change the world, but they won't give me the sources.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140826/99da2d6a/attachment-0001.html>