Hi Richard,
On 09/06/2019 09:28, Reingruber, Richard wrote:
Hi Alex,
that's a good fix for the issue.
One minor thing:
89 Exception error = null;
90 for (int retry = 0; retry < 5; retry++) {
91 try {
92 log("retry: " + retry);
93 s = new Socket("localhost", port);
94 error = null;
95 s.getOutputStream().write("JDWP-".getBytes("UTF-8"));
96 break;
97 } catch (ConnectException ex) {
98 log("got exception: " + ex.toString());
99 error = ex;
100 }
101 }
102 if (error != null) {
103 throw error;
104 }
Is there a reason to clear the local variable error in line 94 instead of
clearing it
in line 91 where each new attempt begins?
The logic here is:
The cycle has 2 exits:
- error (max retry attempts reached, error is set by last "catch")
- success ("break" statement at line 96, error should be null)
So error is cleared only after the socket is connected (this is the
problematic operation which can cause ConnectException).
Of course error can be cleared before each try - there is not functional
difference.
--alex
Cheers, Richard.
-----Original Message-----
From: serviceability-dev <[email protected]> On
Behalf Of [email protected]
Sent: Mittwoch, 4. September 2019 22:11
To: Alex Menkov <[email protected]>; OpenJDK Serviceability
<[email protected]>
Subject: Re: RFR: JDK-8192057: com/sun/jdi/BadHandshakeTest.java fails with
java.net.ConnectException
Hi Alex,
The fix looks good.
Good simplification!
Thanks,
Serguei
On 9/4/19 12:19, Alex Menkov wrote:
Hi all,
Please review the fix for BadHandshakeTest test.
The problem is the test connects to the server twice and if debuggee
hasn't yet handled disconnection, the next connect gets "connection
refused" error.
Instead of adding delay before 2nd connect (we never know "good" value
for the delay and big delay can cause "accept timeout"), the test
re-tries connect in case of ConnectException.
Also improved/simplified the test slightly - debuggee is now run with
auto port assignment (used lib.jdb.Debuggee test class which
implements required functionality).
jira:
https://bugs.openjdk.java.net/browse/JDK-8192057
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/BadHandshakeTest/webrev/
--alex
