Hi Daniil,
It looks good to me.
I noticed a tiny nit in TTY.java where you removed an empty line (l171)
Thanks!
Jc
On Wed, Oct 17, 2018 at 5:50 PM Daniil Titov <daniil.x.ti...@oracle.com
<mailto:daniil.x.ti...@oracle.com>> wrote:
Hi Chris, Alex and JC,
I created a separate issue to deal with "non-atomic" jdb output at
https://bugs.openjdk.java.net/browse/JDK-8212626 .
Please review an updated fix that includes the changes Alex suggested.
Webrev: http://cr.openjdk.java.net/~dtitov/8211736/webrev.04
<http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.04>
Issue: https://bugs.openjdk.java.net/browse/JDK-8211736
Thanks!
--Daniil
On 10/17/18, 5:06 PM, "Daniil Titov" <daniil.x.ti...@oracle.com
<mailto:daniil.x.ti...@oracle.com>> wrote:
Hi Chris,
The previous email was accidentally fired before I managed to
type anything in. Sorry for confusion.
Jdb constantly reads new commands from System.in despite
whether the breakpoint is hit and/or the prompt is printed.
cat -n
src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java
786 // Process interactive commands.
787 MessageOutput.printPrompt();
788 while (true) {
789 String ln = in.readLine();
790 if (ln == null) {
791 /*
792 * Jdb is being shutdown
because debuggee exited, ignore any 'null'
793 * returned by readLine()
during shutdown. JDK-8154144.
794 */
795 if (!isShuttingDown()) {
796
MessageOutput.println("Input stream closed.");
797 }
798 ln = "quit";
799 }
800
801 if (ln.startsWith("!!") && lastLine
!= null) {
802 ln = lastLine + ln.substring(2);
803
MessageOutput.printDirectln(ln);// Special case: use printDirectln()
804 }
805
806 StringTokenizer t = new
StringTokenizer(ln);
807 if (t.hasMoreTokens()) {
808 lastLine = ln;
809 executeCommand(t);
810 } else {
811 MessageOutput.printPrompt();
812 }
813 }
814 } catch (VMDisconnectedException e) {
815 handler.handleDisconnectedException();
816 }
Below is a sample debug session for the following test class
that sends commands "threads" and "quit"
1 public class LoopTest {
2 public static void main(String[] args) throws
Exception {
3 Thread thread = Thread.currentThread();
4 while(true) {
5 print(thread);
6 Thread.sleep(5000);
7 }
8 }
9
10 public static void print(Object obj) {
11 //System.out.println(obj);
12 }
13 }
datitov-mac:work datitov$
~/src/jdk-hs/build/macosx-x64-debug/images/jdk/bin/jdb LoopTest
Initializing jdb ...
> stop go at LoopTest:6
Deferring breakpoint LoopTest:6.
It will be set after the class is loaded.
> run
run LoopTest
Set uncaught java.lang.Throwable
Set deferred uncaught java.lang.Throwable
>
VM Started: Set deferred breakpoint LoopTest:6
Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
6 Thread.sleep(5000);
Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
6 Thread.sleep(5000);
threads
Group system:
(java.lang.ref.Reference$ReferenceHandler)0x172 Reference
Handler running
(java.lang.ref.Finalizer$FinalizerThread)0x173 Finalizer
cond. waiting
(java.lang.Thread)0x174 Signal
Dispatcher running
Group main:
(java.lang.Thread)0x1 main
sleeping
Group InnocuousThreadGroup:
(jdk.internal.misc.InnocuousThread)0x197
Common-Cleaner cond. waiting
>
Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
6 Thread.sleep(5000);
Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
6 Thread.sleep(5000);
quit
Breakpoint hit: "thread=main", LoopTest.main(), line=6 bci=8
6 Thread.sleep(5000);
datitov-mac:work datitov$
I think we could print a simple prompt in this case as Alex
suggested.
Best regards,
Daniil
On 10/17/18, 3:52 PM, "Chris Plummer"
<chris.plum...@oracle.com <mailto:chris.plum...@oracle.com>> wrote:
What is the jdb state for a breakpoint with SUSPEND_NONE?
Can you
actually type in commands even though no threads are suspended?
Chris
On 10/17/18 3:31 PM, Alex Menkov wrote:
> Hi Daniil, Chris,
>
> As far as I understand, the fix does not completely fixes
all
> "non-atomic" output issues (at least TTY.executeCommand
still prints
> prompt separately), so I agree that handle it as separate
issue would
> be better.
> Unfortunately I don't know Gary's ideas for the issue.
>
> About the fix for print prompt:
> 1) TTY.java:
> + // Print current location if suspend policy
is SUSPEND_NONE
> I suppose you mean "Print breakpoint location"
>
> 2) Does it make sense to use printCurrentLocation for
> SUSPEND_EVENT_THREAD?
> Is it expected to print something different from
printBreakpointLocation?
>
> 3) I don't quite understand why we don't print simple
prompt for
> SUSPEND_NONE. IIUC jdb can accept new command in both
> SUSPEND_EVENT_THREAD and SUSPEND_NONE cases (prompt shows
that jdb
> waits for next command, right?)
>
> 4) JdbStopThreadTest.java
> New line line in Java regexp is "\\R"
>
> --alex
>
> On 10/17/2018 10:47, 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/
<http://cr.openjdk.java.net/%7Edtitov/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 <mailto: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/>
>>> >>
<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
<mailto:jcbey...@google.com>>
>>> >> *Date: *Thursday, October 11, 2018 at 7:17 PM
>>> >> *To: *<daniil.x.ti...@oracle.com
<mailto:daniil.x.ti...@oracle.com>>
>>> >> *Cc: *<serviceability-dev@openjdk.java.net
<mailto: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>
>>>
>>> >>
>>>
<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>
>>> <mailto: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>
>>> >>
<http://cr.openjdk.java.net/%7Edtitov/8211736/webrev.01>
>>> >> Issue:
https://bugs.openjdk.java.net/browse/JDK-8211736
>>> >>
>>> >> Thanks!
>>> >> --Daniil
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >>
>>> >> Thanks,
>>> >>
>>> >> Jc
>>> >>
>>> >
>>>
>>>
>>
>>
--
Thanks,
Jc
