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

Thu, 18 Oct 2018 04:46:11 -0700 

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.men...@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 <jcbey...@google.com>
>> *Date: *Thursday, October 11, 2018 at 7:17 PM
>> *To: *<daniil.x.ti...@oracle.com>
>> *Cc: *<serviceability-dev@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.ti...@oracle.com <mailto:daniil.x.ti...@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
>>
>

Reply via email to