[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: JDK-8154144 Tests in com/sun/jdi fails intermittently with "jdb input stream closed prematu
From: "Daniel D. Daugherty" <daniel.daugherty () oracle ! com>
Date: 2016-04-29 14:18:43
Message-ID: 57236D43.1020704 () oracle ! com
[Download RAW message or body]
Thanks for the additional details. I'm good with the
concept of this fix.
> http://cr.openjdk.java.net/~sballal/8154144/webrev.00/
General comment
Please update the copyright years before you push.
src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/EventHandler.java
No comments.
src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java
L763: if(!isShuttingDown()) {
Nit: please add a space between 'if' and '('.
Thumbs up.
Dan
P.S.
Part of me wonders if this line:
L764: MessageOutput.println("Input stream closed.");
should change to:
L764: MessageOutput.println("Input stream closed unexpectedly.");
But that would likely require updating the tests that look
for this pattern. Probably not worth it.
On 4/28/16 11:55 PM, Sharath Ballal wrote:
>
> Hi Dan,
>
> The debugger has received a ‘vmDeathEvent' and as part of handling
> this event, the System.exit() is being called (handleExitEvent() in
> EventHandler.java). So the fact that debuggee has finished executing,
> is known to the debugger and the debugger is explicitly cleaning up
> after that. This is similar to the breakpoint you mentioned. Instead
> of breakpoint, we have an event. Now as part of the explicit cleanup
> the readLine() (which is always blocking) sometimes returns with a
> ‘null'. Since we know we are cleaning up and we also know that the
> debuggee has done executing, I am avoiding printing the error message.
>
> -Sharath Ballal
>
> *From:*Daniel D. Daugherty
> *Sent:* Thursday, April 28, 2016 7:48 PM
> *To:* Sharath Ballal; Staffan Larsen
> *Cc:* serviceability-dev@openjdk.java.net
> *Subject:* Re: RFR: JDK-8154144 Tests in com/sun/jdi fails
> intermittently with "jdb input stream closed prematurely
>
> > When JDB is done, as part of the shutdown it calls System.exit()
> > at Env.java:92 in a different thread 'event-handler'.
>
> Based on the above line from below, I'm even more convinced that
> this bug is hit because the debuggee VM is allowed to run off the
> end of main() and that creates a race between debuggee VM's attempt
> to exit and the debugger trying to control the test execution.
>
> While this fix makes this bug "go away", I don't think this is the
> right solution. You're basically trying to harden our ability to
> handle this race condition instead of closing the race condition.
>
> I'll repeat part of the comment that I added to the bug on 04.13:
>
> > One possible solution is to add a breakpoint after main() returns and change
> > the debuggee <-> debugger protocol to always expect one final breakpoint.
> > Positive enforcement that the test reaches that point and it should catch any
> > accidental "run of the end of main()" issues.
>
> Dan
>
> On 4/28/16 2:25 AM, Sharath Ballal wrote:
>
> Hi Staffan,
>
> The root cause of this problem is that BufferedReader.readLine()
> is intermittently returning 'null' during System.exit(0).
>
> In TTY.java:751 we are always blocking on readLine(). Whenever a
> user enters a command in JDB, the readLine() returns the command,
> which gets executed. This is running in the 'main' thread.
>
> When JDB is done, as part of the shutdown it calls System.exit()
> at Env.java:92 in a different thread 'event-handler'.
>
> Usually (in the passing cases) calling System.exit() is the end of
> the process and readLine() doesn't return 'null', but in the
> failing case readLine() at TTY.java:751 returns 'null'. This
> causes the string "Input stream closed." to be printed. The jtreg
> testcase looks for this string to decide that the testcase failed.
>
> The fix I have done is avoiding the print because we know that we
> are shutting down and hence can discard the ‘null' returned by
> readLine().
>
> Regarding the testing, I have run few of the failing jtreg
> testcases about 200 times and not seen the problem (earlier I was
> able to recreate without the fix). I have to do other tests, but
> sent for review to do it in parallel.
>
> jdk/src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java
>
>
> 708 BufferedReader in =
> 709 new BufferedReader(new InputStreamReader(System.in));
> .......
> .......
> 748 // Process interactive commands.
> 749 MessageOutput.printPrompt();
> 750 while (true) {
> 751 String ln = in.readLine();
> 752 if (ln == null) {
> 753 MessageOutput.println("Input stream closed.");
> 754 ln = "quit";
> 755 }
>
> jdk/src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/Env.java
>
>
> 79 static void shutdown(String message) {
> 80 if (connection != null) {
> 81 try {
> 82 connection.disposeVM();
> 83 } catch (VMDisconnectedException e) {
> 84 // Shutting down after the VM has gone away. This is
> 85 // not an error, and we just ignore it.
> 86 }
> 87 }
> 88 if (message != null) {
> 89 MessageOutput.lnprint(message);
> 90 MessageOutput.println();
> 91 }
> 92 System.exit(0);
> 93 }
>
> -Sharath Ballal
>
> *From:*Staffan Larsen
> *Sent:* Thursday, April 28, 2016 1:17 PM
> *To:* Sharath Ballal
> *Cc:* serviceability-dev@openjdk.java.net
> <mailto:serviceability-dev@openjdk.java.net>
> *Subject:* Re: RFR: JDK-8154144 Tests in com/sun/jdi fails
> intermittently with "jdb input stream closed prematurely
>
> Hi Sharath,
>
> Can you explain more how this help with the problem in the bug?
>
> It looks like you are trying to avoid a race by not printing the
> "Input stream closed." message while shutting down. You added this:
>
> *136 ((TTY)notifier).setShuttingDown(true);*
>
> 137 Env.shutdown(shutdownMessageKey);
>
> The call on line 137 will result in a System.exit(0) call if I am
> reading the code right. So adding the shutdown flag to line 136
> will maybe make the race smaller, but isn't really solving it?
>
> What kind of testing have you run this fix through?
>
> Thanks,
>
> /Staffan
>
> On 28 apr. 2016, at 09:22, Sharath Ballal
> <sharath.ballal@oracle.com <mailto:sharath.ballal@oracle.com>>
> wrote:
>
> Hi,
>
> Pls review the change for bug
>
> JDK-8154144
> <https://bugs.openjdk.java.net/browse/JDK-8154144>- Tests in
> com/sun/jdi fails intermittently with "jdb input stream closed
> prematurely
>
> Webrev:
>
> http://cr.openjdk.java.net/~sballal/8154144/webrev.00/
> <http://cr.openjdk.java.net/%7Esballal/8154144/webrev.00/>
>
> -Sharath Ballal
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<tt>Thanks for the <tt>additional details. I'm good with the<br>
<tt>concept of this fix.<br>
<br>
<tt>>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~sballal/8154144/webrev.00/">http://cr.openjdk.java.net/~sballal/8154144/webrev.00/</a><br>
<br>
<tt>General comment<br>
<tt> Please update the copyright years before you push.<br>
<br>
</tt></tt><tt>src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/EventHandler.java<br>
<tt> No comments.<br>
<br>
</tt>src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java<br>
<tt> L763: \
if(!isShuttingDown()) {<br>
<tt> Nit<tt>: please add a sp<tt>ace between 'if'
and '('<tt>.<br>
<br>
<tt>Thumbs up<tt>.<br>
<br>
<tt>Dan<br>
<br>
<tt>P.S.<br>
<tt>Part of me wonders if this line:<br>
<br>
<tt>L764<tt>:</tt>
MessageOutput.println("Input stream
closed.");<br>
<br>
<tt>should ch<tt>ange to:<br>
<br>
L764:
MessageOutput.println("Input
stream closed unexpectedly.");<br>
<br>
<tt>But that would likely
require updati<tt>ng the tests
that look<br>
<tt>for this pattern.
Probably not worth it.</tt><br>
\
</tt></tt></tt></tt></tt></tt></tt></tt></tt></tt></tt></tt></tt></tt></tt></tt><br> \
</tt><br> </tt></tt></tt><br>
<div class="moz-cite-prefix">On 4/28/16 11:55 PM, Sharath Ballal
wrote:<br>
</div>
<blockquote cite="mid:890dc06b-99b7-4105-b734-9d9df227d74c@default"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 12 (filtered
medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";
color:black;}
tt
{mso-style-priority:99;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;}
span.new
{mso-style-name:new;}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle22
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.EmailStyle23
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi
Dan,<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> \
</o:p></span></p> <p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The
debugger has received a ‘vmDeathEvent' and as part of
handling this event, the System.exit() is being called
(handleExitEvent() in EventHandler.java). So the fact that
debuggee has finished executing, is known to the debugger
and the debugger is explicitly cleaning up after that. This
is similar to the breakpoint you mentioned. Instead of
breakpoint, we have an event. Now as part of the explicit
cleanup the readLine() (which is always blocking) sometimes
returns with a ‘null'. Since we know we are cleaning up and
we also know that the debuggee has done executing, I am
avoiding printing the error message.<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> \
</o:p></span></p> <div>
<p class="MsoNormal"><span
style="font-family:"Calibri","sans-serif";color:#1F497D">-Sharath
Ballal<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-family:"Calibri","sans-serif";color:#1F497D"><o:p> \
</o:p></span></p> </div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> \
</o:p></span></p> <div>
<div style="border:none;border-top:solid #B5C4DF
1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:windowtext">From:</span></b><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:windowtext">
Daniel D. Daugherty <br>
<b>Sent:</b> Thursday, April 28, 2016 7:48 PM<br>
<b>To:</b> Sharath Ballal; Staffan Larsen<br>
<b>Cc:</b> <a class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><br>
<b>Subject:</b> Re: RFR: JDK-8154144 Tests in
com/sun/jdi fails intermittently with "jdb input stream
closed prematurely<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><tt><span
style="font-size:10.0pt">> When JDB is done, as part of
the shutdown it calls System.exit()</span></tt><span
style="font-size:10.0pt;font-family:"Courier New""><br>
<tt>> at Env.java:92 in a different thread
'event-handler'.</tt><br>
<br>
<tt>Based on the above line from below, I'm even more
convinced that</tt><br>
<tt>this bug is hit because the debuggee VM is allowed to
run off the</tt><br>
<tt>end of main() and that creates a race between debuggee
VM's attempt</tt><br>
<tt>to exit and the debugger trying to control the test
execution.</tt><br>
<br>
<tt>While this fix makes this bug "go away", I don't think
this is the</tt><br>
<tt>right solution. You're basically trying to harden our
ability to</tt><br>
<tt>handle this race condition instead of closing the race
condition.</tt><br>
<br>
<tt>I'll repeat part of the comment that I added to the bug
on 04.13:</tt><br>
<br>
<tt>> One possible solution is to add a breakpoint after
main() returns and change</tt><br>
<tt>> the debuggee <-> debugger protocol to always
expect one final breakpoint.</tt><br>
<tt>> Positive enforcement that the test reaches that
point and it should catch any</tt><br>
<tt>> accidental "run of the end of main()" issues. </tt><br>
<br>
<tt>Dan</tt><br>
<br>
</span><o:p></o:p></p>
<div>
<p class="MsoNormal">On 4/28/16 2:25 AM, Sharath Ballal \
wrote:<o:p></o:p></p> </div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal">Hi Staffan,<o:p></o:p></p>
<p class="MsoNormal">The root cause of this problem is that
BufferedReader.readLine() is intermittently returning 'null'
during System.exit(0). <br>
<br>
In TTY.java:751 we are always blocking on readLine().
Whenever a user enters a command in JDB, the readLine()
returns the command, which gets executed. This is running in
the 'main' thread. <br>
<br>
When JDB is done, as part of the shutdown it calls
System.exit() at Env.java:92 in a different thread
'event-handler'. <br>
<br>
Usually (in the passing cases) calling System.exit() is the
end of the process and readLine() doesn't return 'null', but
in the failing case readLine() at TTY.java:751 returns
'null'. This causes the string "Input stream closed." to be
printed. The jtreg testcase looks for this string to decide
that the testcase failed. <o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">The fix I have done is avoiding the print
because we know that we are shutting down and hence can
discard the ‘null' returned by readLine().<o:p></o:p></p>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Regarding the testing, I have run few of
the failing jtreg testcases about 200 times and not seen the
problem (earlier I was able to recreate without the fix). I
have to do other tests, but sent for review to do it in
parallel.<br>
<br>
jdk/src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java
<br>
<br>
708 BufferedReader in = <br>
709 new BufferedReader(new InputStreamReader(System.in)); <br>
....... <br>
....... <br>
748 // Process interactive commands. <br>
749 MessageOutput.printPrompt(); <br>
750 while (true) { <br>
751 String ln = in.readLine(); <br>
752 if (ln == null) { <br>
753 MessageOutput.println("Input stream closed."); <br>
754 ln = "quit"; <br>
755 } <br>
<br>
jdk/src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/Env.java
<br>
<br>
79 static void shutdown(String message) { <br>
80 if (connection != null) { <br>
81 try { <br>
82 connection.disposeVM(); <br>
83 } catch (VMDisconnectedException e) { <br>
84 // Shutting down after the VM has gone away. This is <br>
85 // not an error, and we just ignore it. <br>
86 } <br>
87 } <br>
88 if (message != null) { <br>
89 MessageOutput.lnprint(message); <br>
90 MessageOutput.println(); <br>
91 } <br>
92 System.exit(0); <br>
93 }<o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> \
</span><o:p></o:p></p> <div>
<p class="MsoNormal"><span
style="font-family:"Calibri","sans-serif";color:#1F497D"> \
</span><o:p></o:p></p> <p class="MsoNormal"><span
style="font-family:"Calibri","sans-serif";color:#1F497D">-Sharath
Ballal</span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-family:"Calibri","sans-serif";color:#1F497D"> \
</span><o:p></o:p></p> </div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> \
</span><o:p></o:p></p> <div>
<div style="border:none;border-top:solid #B5C4DF
1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span
style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">
Staffan Larsen <br>
<b>Sent:</b> Thursday, April 28, 2016 1:17 PM<br>
<b>To:</b> Sharath Ballal<br>
<b>Cc:</b> <a moz-do-not-send="true"
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><br>
<b>Subject:</b> Re: RFR: JDK-8154144 Tests in
com/sun/jdi fails intermittently with "jdb input
stream closed prematurely</span><o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"> <o:p></o:p></p>
<p class="MsoNormal">Hi Sharath,<o:p></o:p></p>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Can you explain more how this help with
the problem in the bug? <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">It looks like you are trying to avoid a
race by not printing the "Input stream closed." message
while shutting down. You added this:<o:p></o:p></p>
</div>
<div>
<pre><span class="new"><b><span style="color:blue"> 136 \
((TTY)notifier).setShuttingDown(true);</span></b></span><o:p></o:p></pre>
<pre> 137 \
Env.shutdown(shutdownMessageKey);<o:p></o:p></pre> <p class="MsoNormal">The call on \
line 137 will result in a System.exit(0) call if I am reading the code right.
So adding the shutdown flag to line 136 will maybe make
the race smaller, but isn't really solving it?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">What kind of testing have you run this
fix through?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
<div>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">/Staffan<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"> <o:p></o:p></p>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On 28 apr. 2016, at 09:22,
Sharath Ballal <<a moz-do-not-send="true"
\
href="mailto:sharath.ballal@oracle.com">sharath.ballal@oracle.com</a>> \
wrote:<o:p></o:p></p> </div>
<p class="MsoNormal"> <o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Hi,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif""> \
</span><o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Pls
review the change for bug<span
class="apple-converted-space"> \
</span></span><o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif""> \
</span><o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif""><a
moz-do-not-send="true"
\
href="https://bugs.openjdk.java.net/browse/JDK-8154144"><span
style="color:purple">JDK-8154144</span></a><span
class="apple-converted-space"> </span>- Tests
in com/sun/jdi fails intermittently with "jdb
input stream closed prematurely</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif""> \
</span><o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Webrev:</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:purple"><a
moz-do-not-send="true"
\
href="http://cr.openjdk.java.net/%7Esballal/8154144/webrev.00/"><a \
class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~sballal/8154144/webrev.00/">http://cr.openjdk.java.net/~sballal/8154144/webrev.00/</a></a></span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span
\
style="font-family:"Calibri","sans-serif""> \
</span><o:p></o:p></p> </div>
<div>
<p class="MsoNormal"><span
\
style="font-family:"Calibri","sans-serif"">-Sharath \
Ballal</span><o:p></o:p></p> </div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic