RFR(XXS): 8213902: com/sun/jdi/SetLocalWhileThreadInNative.java times out

[David Holmes]

Hi Richard,

I'll sponsor this over the weekend.

Thanks,
David

On 17/11/2018 1:37 am, Reingruber, Richard wrote:
> Hi Jc, David, Chris,
> 
>    >> Is this not a test that's failing due to a real bug?
> 
> I would not consider this a bug, but rather a limitation. I don't see a way to
> work around it in the TestScaffold, though (looking at [1]).
> 
> So an enhancement would have to happen in the jdi implementation. To me it does
> not seem worth the effort. I'd say the tracing is not necessary for regression
> testing. In the case of a test error where all that output is required, I'd
> start the debuggee in a shell and do the debugging manually.
> 
> Regarding -Xcomp: makes a lot of sense for tests that are indifferent about the
> execution mode. The test at hand triggers compilation of the central test
> method. It does not help that much, to compile everything else, too.
> 
> Anyway, here is another webrev without excluding -Xcomp:
> 
> http://cr.openjdk.java.net/~rrich/webrevs/2018/8213902/webrev.02/
> 
> I'd prefer the original webrev, because it's greener ;)
> 
> I would need a sponsor, too. Please take either one of the patches.
> 
> Thanks for reviewing,
> Richard.
> 
> [1] Debugger stack
> 
> java.lang.Object.wait(long) line: not available [native method]
> com.sun.tools.jdi.AbstractLauncher$Helper(java.lang.Object).wait() line: 328
> com.sun.tools.jdi.AbstractLauncher$Helper.launchAndAccept() line: 197
> com.sun.tools.jdi.SunCommandLineLauncher(com.sun.tools.jdi.AbstractLauncher).launch(java.lang.String[], java.lang.String, com.sun.jdi.connect.spi.TransportService$ListenKey, com.sun.jdi.connect.spi.TransportService) line: 140	
> com.sun.tools.jdi.SunCommandLineLauncher.launch(java.util.Map<java.lang.String,? extends com.sun.jdi.connect.Connector.Argument>) line: 229	
> VMConnection.launchTarget() line: 314
> VMConnection.open() line: 149
> SetLocalWhileThreadInNative(TestScaffold).connect(java.lang.String[]) line: 637
> SetLocalWhileThreadInNative(TestScaffold).startUp(java.lang.String) line: 364
> SetLocalWhileThreadInNative(TestScaffold).startTo(java.lang.String, java.lang.String, java.lang.String) line: 374	
> SetLocalWhileThreadInNative(TestScaffold).startToMain(java.lang.String) line: 369	
> SetLocalWhileThreadInNative.runTests() line: 132
> SetLocalWhileThreadInNative(TestScaffold).startTests() line: 431
> SetLocalWhileThreadInNative.main(java.lang.String[]) line: 120
> 
> -----Original Message-----
> From: Chris Plummer <chris.plummer at oracle.com>
> Sent: Freitag, 16. November 2018 06:23
> To: David Holmes <david.holmes at oracle.com>; JC Beyler <jcbeyler at google.com>; Reingruber, Richard <richard.reingruber at sap.com>
> Cc: serviceability-dev at openjdk.java.net
> Subject: Re: RFR(XXS): 8213902: com/sun/jdi/SetLocalWhileThreadInNative.java times out
> 
> See https://bugs.openjdk.java.net/browse/JDK-8173304 and also
> https://bugs.openjdk.java.net/browse/JDK-4368399, which dates back to 2000.
> 
> Chris
> 
> On 11/15/18 8:40 PM, David Holmes wrote:
>> Hi Jc,
>>
>> If I may put in my 2c ...
>>
>> On 16/11/2018 2:13 pm, JC Beyler wrote:
>>> Hi Richard,
>>>
>>> Is this not a test that's failing due to a real bug? Ie: if someone
>>> were to really be running the program in this configuration (with
>>> printcompilation and printinlining) and attaching a debugger, would
>>> they not reach the same issue?
>>
>> This is just a limitation of connecting process streams via finite
>> pipes. If the pipe can fill before the target process reaches the
>> point where it communicates back to the initial process (which would
>> trigger the initial process to start draining the pipe) then you
>> effectively deadlock.
>>
>> So you either structure the tests so that they work correctly with the
>> finite pipes provided by the test scaffold, or you modify the test
>> scaffold to either provide a bigger pipe or else rework it to ensure
>> pipes can't fill and cause indefinite blocking.
>>
>> For the case at hand fixing the test to not be so verbose is
>> reasonable in my opinion, though I'm not sure I'd remove Xcomp
>> altogether as Xcomp likes a range of programs to put it through its
>> paces.
>>
>> But it wouldn't be unreasonable to file a RFE to make the TestScaffold
>> handle this better.
>>
>> Cheers,
>> David
>>
>>> Should that not be fixed in some way instead? Or is this such an
>>> improbable case that we just consider it ludicrous?
>>>
>>> Thanks,
>>> Jc
>>>
>>> On Thu, Nov 15, 2018 at 2:32 PM Reingruber, Richard
>>> <richard.reingruber at sap.com <mailto:richard.reingruber at sap.com>> wrote:
>>>
>>>      Hi,
>>>
>>>      could I please get reviews for the following small patch? It fixes a
>>>      bug in the test
>>>      com/sun/jdi/SetLocalWhileThreadInNative.java that causes a deadlock
>>>      when executed with
>>>      -vmoption:-Xcomp.
>>>
>>>      Deadlock:
>>>
>>>      Debuggee (SetLocalWhileThreadInNativeTarget):
>>>         - running with -Xcomp
>>>         - still in early start-up
>>>         - printed a lot on tty already, because -XX:+PrintCompilation
>>>      -XX:+PrintInlining are given
>>>         - java thread waits for compiler thread to finish compile task
>>>         - compiler thread is blocked in write on tty. tty buffer is full,
>>>      because debugger is not yet
>>>           reading debuggee's output
>>>
>>>      Debugger
>>>         - waiting until connection to debugger is established
>>>
>>>      The fix is to remove -XX:+PrintCompilation -XX:+PrintInlining. In
>>>      addition it excludes the test if
>>>      running with -Xcomp, because that mode does not add a lot of value,
>>>      as the test actually only needs
>>>      dontinline_testMethod() to be compiled. Running with -Xcomp just
>>>      wastes energy.
>>>
>>>      Webrev:
>>> http://cr.openjdk.java.net/~rrich/webrevs/2018/8213902/webrev.01/
>>>      Bug: https://bugs.openjdk.java.net/browse/JDK-8213902
>>>
>>>      The contribution needs to be sponsored as well, please.
>>>
>>>      Thanks, Richard.
>>>
>>>
>>>
>>> -- 
>>>
>>> Thanks,
>>> Jc
> 
> 
>