RFR 8211736: jdb doesn't print prompt when breakpoint is hit and suspend policy is STOP_EVENT_THREAD

[Chris Plummer]

+1

On 10/18/18 4:47 AM, Gary Adams wrote:
> Back in July, I tried to address the jdb output synchronization problem
> by buffering the output from commands and events.
>
>   http://cr.openjdk.java.net/~gadams/8169718/webrev.01/
>
> The changes were very invasive and only provided a partial solution.
>
> Bottom line for me is the fact that the jdb commands, replies and prompts
> were never designed as a rigid protocol. They were designed to be 
> interactive
> with a user and guiding them through a debugging session.
>
> The tests that process jdb commands are fragile, because they run
> much quicker than user interactive speeds, so race conditions can be 
> observed.
> Also, the reliance on a single standard output stream allows for 
> interspersed
> output.
>
> I think we are pretty close to reaching the current test stabilization 
> goals and
> this area doesn't really merit additional investment at this time.
>
> On 10/17/18, 1:47 PM, Chris Plummer wrote:
>> Hi Alex,
>>
>> I think the tty buffering should be a separate bug fix, and I'd also 
>> like input from Gary on it since he had tried something similar at 
>> one point. It should probably also include a multithread test to 
>> prove the fix is working (after first getting the test to fail 
>> without the changes).
>>
>> thanks,
>>
>> Chris
>>
>> On 10/16/18 8:59 PM, Daniil Titov wrote:
>>> Hi Alex, Chris and JC,
>>>
>>> Please review an updated version of the patch that addresses these 
>>> issues.
>>>
>>> Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.03/
>>> Issue:  https://bugs.openjdk.java.net/browse/JDK-8211736
>>>
>>> Thanks!
>>> --Daniil
>>>
>>>
>>> On 10/12/18, 9:52 AM, "Alex Menkov" <alexey.menkov at oracle.com> wrote:
>>>
>>>      Hi Daniil,
>>>           1) +1 for printCurrentLocation when suspendPolicy != 
>>> SUSPEND_ALL
>>>           2) wrong indent in JdbStopThreadTest.java:
>>>         36         import lib.jdb.JdbCommand;
>>>         37         import lib.jdb.JdbTest;
>>>           3) I think it would be better to make waitForSimplePrompt 
>>> property of
>>>      JdbCommand (like JdbCommand.allowExit)
>>>           jdb.command(JdbCommand.run().replyIsSimplePrompt());
>>>      looks much clearer than
>>>      jdb.command(JdbCommand.run(), true);
>>>           4) (TTY.java)
>>>          MessageOutput.lnprint("Breakpoint hit:");
>>>      +  // Print current location and prompt if suspend policy is
>>>      SUSPEND_EVENT_THREAD.
>>>      +  // In case of SUSPEND_ALL policy this is handled by 
>>> vmInterrupted()
>>>      method.
>>>      +  if (be.request().suspendPolicy() == 
>>> EventRequest.SUSPEND_EVENT_THREAD) {
>>>      + printCurrentLocation(ThreadInfo.getThreadInfo(be.thread()));
>>>      +      MessageOutput.printPrompt();
>>>      +  }
>>>      We have 3 separate outputs.
>>>      If we have several concurrent threads, we can easy get mess of 
>>> outputs
>>>      from different threads.
>>>      I think it would be better to print everything in a single chunk.
>>>      I suppose TTY has other places with similar "non-atomic" 
>>> output, so
>>>      maybe revising TTY output should be handled by separate issue.
>>>           --alex
>>>           On 10/11/2018 22:00, Chris Plummer wrote:
>>> > Hi Daniil,
>>> >
>>> > Can you send output from an example jdb session?
>>> >
>>> > Do you think maybe we should also call printCurrentLocation() when 
>>> the
>>> > suspend policy is NONE?
>>> >
>>> > There's a typo in the following line. The space is on the wrong 
>>> side of
>>> > the comma.
>>> >
>>> >    72 jdb.command(JdbCommand.stopThreadAt(DEBUGGEE_CLASS ,bpLine));
>>> >
>>> > thanks,
>>> >
>>> > Chris
>>> >
>>> > On 10/11/18 8:02 PM, Daniil Titov wrote:
>>> >>
>>> >> Thank you,  JC!
>>> >>
>>> >> Please review an updated version of the patch that fixes newly added
>>> >> test for Windows platform  (now it uses system dependent line
>>> >> separator string).
>>> >>
>>> >> Webrev:http://cr.openjdk.java.net/~dtitov/8211736/webrev.02/
>>> >> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.02/>
>>> >>
>>> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8211736
>>> >>
>>> >> Best regards,
>>> >>
>>> >> --Daniil
>>> >>
>>> >> *From: *JC Beyler <jcbeyler at google.com>
>>> >> *Date: *Thursday, October 11, 2018 at 7:17 PM
>>> >> *To: *<daniil.x.titov at oracle.com>
>>> >> *Cc: *<serviceability-dev at openjdk.java.net>
>>> >> *Subject: *Re: RFR 8211736: jdb doesn't print prompt when breakpoint
>>> >> is hit and suspend policy is STOP_EVENT_THREAD
>>> >>
>>> >> Hi Daniil,
>>> >>
>>> >> Looks good to me. I saw a really small nit:
>>> >>
>>> >> 
>>> http://cr.openjdk.java.net/~dtitov/8211736/webrev.01/test/jdk/com/sun/jdi/JdbStopThreadTest.java.html
>>> >> 
>>> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.01/test/jdk/com/sun/jdi/JdbStopThreadTest.java.html>
>>> >>
>>> >> 70  jdb.command(JdbCommand.stopThreadAt( DEBUGGEE_CLASS ,bpLine));
>>> >>
>>> >> There is a space after the '('.
>>> >>
>>> >> No need to send a new webrev for that evidently :),
>>> >>
>>> >> Jc
>>> >>
>>> >> On Thu, Oct 11, 2018 at 5:07 PM Daniil Titov
>>> >> <daniil.x.titov at oracle.com <mailto:daniil.x.titov at oracle.com>> 
>>> wrote:
>>> >>
>>> >>     Please review the change that fixes the issue with missing 
>>> prompt
>>> >>     in jdb when a breakpoint is hit and the suspend policy is set to
>>> >>     stop the thread only.
>>> >>
>>> >>     Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.01
>>> >> <http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.01>
>>> >>     Issue: https://bugs.openjdk.java.net/browse/JDK-8211736
>>> >>
>>> >>     Thanks!
>>> >>     --Daniil
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >>
>>> >> Thanks,
>>> >>
>>> >> Jc
>>> >>
>>> >
>>>
>>>
>>
>>
>