Hi Kurchi,
i looked into the change. Today i did run it a few time and it allays
worked. The logic looks straight forward to me.
I would only improve indenting and the try - autoclose for the Server.
The startServer() can be implemented in the constructor. The
server.close() in the exept() method is not required due to the
autoclose and this saves a few parameters here and there.
cheers
Andreas
Here my diff on your patch change:
diff --git a/test/java/net/Authenticator/B4769350.java
b/test/java/net/Authenticator/B4769350.java
--- a/test/java/net/Authenticator/B4769350.java
+++ b/test/java/net/Authenticator/B4769350.java
@@ -55,7 +55,7 @@
}
}
- static class Client extends Thread {
+ static class Client extends Thread {
String authority, path;
boolean allowerror;
@@ -74,7 +74,7 @@
InputStream is = urlc.getInputStream();
read (is);
is.close();
- } catch (URISyntaxException e) {
+ } catch (URISyntaxException e) {
System.out.println (e);
error = true;
} catch (IOException e) {
@@ -94,11 +94,7 @@
CyclicBarrier t1Cond1;
CyclicBarrier t1Cond2;
- public String getAddress() {
- return server.getAddress().getHostName();
- }
-
- public void startServer() {
+ public Server() {
InetSocketAddress addr = new InetSocketAddress(0);
try {
@@ -131,6 +127,10 @@
server.start();
}
+ public String getAddress() {
+ return server.getAddress().getHostName();
+ }
+
public int getPort() {
return server.getAddress().getPort();
}
@@ -143,9 +143,10 @@
}
/* T1 tests the client by sending 4 requests to 2 different realms
- * in parallel. The client should recognise two pairs of
dependent requests
- * and execute the first of each pair in parallel. When they
both succeed
- * the second requests should be executed without calling the
authenticator.
+ * in parallel. The client should recognise two pairs of dependent
+ * requests and execute the first of each pair in parallel.
When they
+ * both succeed the second requests should be executed without
calling
+ * the authenticator.
* The test succeeds if the authenticator was only called twice.
*/
class AuthenticationHandlerT1a implements HttpHandler
@@ -171,11 +172,9 @@
default:
System.out.println ("Unexpected request");
}
- } catch (InterruptedException |
- BrokenBarrierException e)
- {
- throw new RuntimeException(e);
- }
+ } catch (InterruptedException | BrokenBarrierException e) {
+ throw new RuntimeException(e);
+ }
}
}
@@ -270,10 +269,9 @@
}
}
- /* T2 tests to check that if initial authentication fails, the
second will
- * succeed, and the authenticator is called twice
+ /* T2 tests to check that if initial authentication fails, the
second
+ * will succeed, and the authenticator is called twice
*/
-
class AuthenticationHandlerT2a implements HttpHandler
{
volatile int count = -1;
@@ -287,11 +285,10 @@
}
AuthenticationHandler.errorReply(exchange,
"Basic realm=\"realm3\"");
-
}
}
- class AuthenticationHandlerT2b implements HttpHandler
+ class AuthenticationHandlerT2b implements HttpHandler
{
volatile int count = -1;
@@ -316,7 +313,6 @@
* resource at same time. Authenticator should be called once
for server
* and once for proxy
*/
-
class AuthenticationHandlerT3a implements HttpHandler
{
volatile int count = -1;
@@ -423,11 +419,10 @@
int f = auth.getCount();
if (f != 2) {
- except ("Authenticator was called "+f+" times. Should be 2",
- server);
+ except ("Authenticator was called "+f+" times. Should be 2");
}
if (error) {
- except ("error occurred", server);
+ except ("error occurred");
}
auth.resetCount();
@@ -444,10 +439,10 @@
f = auth.getCount();
if (f != redirects+1) {
except ("Authenticator was called "+f+" times. Should be: "
- + redirects+1, server);
+ + redirects+1);
}
if (error) {
- except ("error occurred", server);
+ except ("error occurred");
}
}
@@ -466,11 +461,10 @@
int f = auth.getCount();
if (f != 2) {
- except ("Authenticator was called "+f+" times. Should be: "
+ 2,
- server);
+ except ("Authenticator was called "+f+" times. Should be: "
+ 2);
}
if (error) {
- except ("error occurred", server);
+ except ("error occurred");
}
}
@@ -482,10 +476,11 @@
System.setProperty ("http.maxRedirects", Integer.toString
(redirects));
System.setProperty ("http.auth.serializeRequests", "true");
Authenticator.setDefault (auth);
+
try (Server server = new Server()) {
- server.startServer();
System.out.println ("Server: listening on port: "
+ server.getPort());
+
if (proxy) {
System.setProperty ("http.proxyHost", "localhost");
System.setProperty ("http.proxyPort",
@@ -495,11 +490,9 @@
doServerTests ("localhost:"+server.getPort(), server);
}
}
-
}
- public static void except (String s, Server server) {
- server.close();
+ public static void except (String s) {
throw new RuntimeException (s);
}
On 25.07.13 20:35, Kurchi Subhra Hazra wrote:
Hi,
Did anyone have a chance to look at this?
Thanks,
Kurchi
On Thu, Jul 18, 2013 at 4:26 PM, Kurchi Hazra
<kurchi.subhra.ha...@oracle.com <mailto:kurchi.subhra.ha...@oracle.com>>
wrote:
Hi Michael,
I added some comments as to what is the purpose of the latches
and barriers. Those comments alongwith the
comments describing the purpose of the handlers should make the
synchronization logic more clear. Let me know what
you think: http://cr.openjdk.java.net/~__khazra/8017779/webrev.01/
<http://cr.openjdk.java.net/~khazra/8017779/webrev.01/>
Thanks,
Kurchi
On 7/17/2013 2:07 PM, Kurchi Hazra wrote:
On 7/17/2013 12:27 AM, Michael McMahon wrote:
On 16/07/13 20:11, Kurchi Hazra wrote:
Hi,
We have observed that
test/java/net/Authenticator/__B4769350.java fails
intermittently. Although not reproducible always,
the bug could be in the test/sun/net/www/httptest
library that this test uses. I have rewritten the test
to use
com.sun.net.httpserver instead since we anyway want to
move away from using the httptest library.
I have used CyclicBarriers to mimic
TestHttpServer.rendezvous() and CountDownLatches to
mimic TestHttpServer.__waitForCondition() and hopefully
preserved the rest of the logic in the test.
I have not seen the test failing after these changes.
Bug: http://bugs.sun.com/view_bug.__do?bug_id=8017779
<http://bugs.sun.com/view_bug.do?bug_id=8017779>
Webrev:
http://cr.openjdk.java.net/~__khazra/8017779/webrev.00/
<http://cr.openjdk.java.net/~khazra/8017779/webrev.00/>
Thanks,
Kurchi
Kurchi,
Since this is a fairly complicated test, and it's great to
see it being rewritten,
is there any possibility of adding some commentary that
explains the purpose
of the synchronization code. For instance, I can't see the
purpose of the call
on line 163 as it just blocks a thread that has already
completed its work
Michael
Hi Michael,
I have just preserved whatever logic the test originally had.
The specific instance you point out waits
for the T1b() handle to be executed for case 0 before moving
forward. But I guess it is hard to understand in a
glance and I'll add some more comments and get back with a new
webrev.
Thanks,
Kurchi
--
-Kurchi
