Re: RFR: JDK-8169718: nsk/jdb/locals/locals002: ERROR: Cannot find boolVar with expected value: false

[Chris Plummer]

Hi Gary, Sorry about the delay in the review. Been trying to fully understand all the various code flows, which is tricky due to having the separate event thread whose output needs to be co-ordinated with command thread. Also made more complicated by the multiple entrypoints into executeCommand(), and there being various places where output produced and we care about the order. I think I have it all mapped out now and a pretty good understanding of how it all works. My first impression is that it seems like a large number of changes and a lot of passing around of a StringBuffer object to fix a problem which I think could be resolved with some strategic wait/notify code. For example, it might be as simple as doing something like this: http://cr.openjdk.java.net/~cjplummer/8169718/webrev.00/ This only covers the step command. You'd have to figure out which other commands need to ensure an event has been processed before printing the prompt, and set waitForEventCompletion = true for them. Is there a reason not to go with something like this? As for you current changes, just a couple of things I've noticed, but I have not given it a thorough review (for example, making sure that the StringBuffer is always eventually printed, and done so in the right order). In EventHandler.run(), it's unclear who is handling the printing of sb when handleEvent() or handleDisconnectedException() are called. In the following EventHandler code:  123     @Override  124     public void stepEvent(StepEvent se, StringBuilder sb)  {  125         Thread.yield();  // fetch output  126         sb.append(MessageOutput.format("Step completed:"));  127     } Do you understand why the Thread.yield() is there? What does the comment mean by "fetch output"? Just seems like it has something to do with the ordering of output, but it seems bad that the code ever relied on a yield to make sure the output was ordered properly. Also in the above code snippet, you need to be careful when replacing MessageOutput.lnprint() with MessageOutput.format(). lnprint() adds a newline to the start of the line. That appears to be missing in your code above. thanks, Chris On 7/20/18 12:11 PM, Gary Adams wrote: Here's another attempt to clear up the overlapping output from the command processing and event handler in the jdb tests. The fundamental problem is observed when "prompts" are produced interleaved with command and event output. This attempts to fix the issue by buffering the output and printing it fully assembled.  Webrev: http://cr.openjdk.java.net/~gadams/8169718/webrev.01/ On 5/26/18, 6:50 AM, gary.ad...@oracle.com wrote: This is a review request for a previously closed test bug. The test was recently moved to the open repos, and the proposed fix is in the open code.   Webrev: http://cr.openjdk.java.net/~gadams/8169718/webrev/ -------- Forwarded Message -------- Subject: RFR: JDK-8169718: nsk/jdb/locals/locals002: ERROR: Cannot find boolVar with expected value: false Date: Fri, 25 May 2018 11:35:10 -0400 From: Gary Adams <gary.ad...@oracle.com> Reply-To: gary.ad...@oracle.com The jdb tests use stdin to send commands to a jdb process and parses the stdout to determine if a command was successful and when the process is prompting for new commands to be sent. Some commands are synchronous, so when the command is completed a new prompt is sent back immediately. Some commands are asynchronous, so there could be a delay until a breakpoint is reached. The event handler then sends a prompt when the application thread is stopped and new jdb commands can be sent. The problem causing the intermittent failures was a corruption in the output stream when prompts were being sent at the wrong times. Instead of receiving "Breakpoint hit:" <location> <prompt> the log contained "Breakpoint hit:" <prompt> <location> Once out of sync, jdb commands were being sent prematurely and the wrong values were being compared against expected behavior. The simple fix proposed here recognizes that commands like "cont", "step" and "next" are asynchronous commands and should not send back a prompt immediately. Instead. the event handler will deliver the next prompt when the next "Breakpoint hit:" or "Step completed:" state change occurs. The bulk of the testing was done on windows-x64-debug builds where the intermittent failures were observed in ~5 in 1000 testruns. The fix has also been tested on linux-x64-debug, solaris-sparcv9-debug, and macosx-x64-debug, even though the failures have never been reported against those platforms. Failures have been observed in many of the nsk/jdb tests with similar corrupted output streams, but never directly associated with this issue before.  redefine001, caught_exception002, locals002, eval001, next001, stop_at003, step002, print002, trace001, step_up001, read001, clear004, kill001, set001