From c74999f00c85f93104ff6fdea7d055e740b5eab9 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 4 Apr 2017 11:26:57 -0400 Subject: [PATCH] Found a race condition which could explain random CI hangs of CLIActionTest.authenticate. --- cli/src/main/java/hudson/cli/CLI.java | 15 +++++++++++---- core/src/main/java/hudson/cli/CLIAction.java | 17 ++++++++++++----- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/cli/src/main/java/hudson/cli/CLI.java b/cli/src/main/java/hudson/cli/CLI.java index d29ea675f4..bce0683679 100644 --- a/cli/src/main/java/hudson/cli/CLI.java +++ b/cli/src/main/java/hudson/cli/CLI.java @@ -712,6 +712,7 @@ public class CLI implements AutoCloseable { URL jenkins = new URL(url + "cli?remoting=false"); FullDuplexHttpStream streams = new FullDuplexHttpStream(jenkins, factory.authorization); class ClientSideImpl extends PlainCLIProtocol.ClientSide { + boolean complete; int exit = -1; ClientSideImpl(InputStream is, OutputStream os) throws IOException { super(is, os); @@ -720,9 +721,9 @@ public class CLI implements AutoCloseable { } } @Override - protected synchronized void onExit(int code) { + protected void onExit(int code) { this.exit = code; - notifyAll(); + finished(); } @Override protected void onStdout(byte[] chunk) throws IOException { @@ -733,7 +734,11 @@ public class CLI implements AutoCloseable { System.err.write(chunk); } @Override - protected synchronized void handleClose() { + protected void handleClose() { + finished(); + } + private synchronized void finished() { + complete = true; notifyAll(); } } @@ -761,7 +766,9 @@ public class CLI implements AutoCloseable { } }.start(); synchronized (connection) { - connection.wait(); + while (!connection.complete) { + connection.wait(); + } } return connection.exit; } diff --git a/core/src/main/java/hudson/cli/CLIAction.java b/core/src/main/java/hudson/cli/CLIAction.java index 15fefb75bb..402a287e95 100644 --- a/core/src/main/java/hudson/cli/CLIAction.java +++ b/core/src/main/java/hudson/cli/CLIAction.java @@ -135,6 +135,7 @@ public class CLIAction implements UnprotectedRootAction, StaplerProxy { protected void run(InputStream upload, OutputStream download) throws IOException, InterruptedException { final AtomicReference runningThread = new AtomicReference<>(); class ServerSideImpl extends PlainCLIProtocol.ServerSide { + boolean ready; List args = new ArrayList<>(); Locale locale = Locale.getDefault(); Charset encoding = Charset.defaultCharset(); @@ -167,8 +168,8 @@ public class CLIAction implements UnprotectedRootAction, StaplerProxy { } } @Override - protected synchronized void onStart() { - notifyAll(); + protected void onStart() { + ready(); } @Override protected void onStdin(byte[] chunk) throws IOException { @@ -179,18 +180,24 @@ public class CLIAction implements UnprotectedRootAction, StaplerProxy { stdinMatch.close(); } @Override - protected synchronized void handleClose() { - notifyAll(); + protected void handleClose() { + ready(); Thread t = runningThread.get(); if (t != null) { t.interrupt(); } } + private synchronized void ready() { + ready = true; + notifyAll(); + } } try (ServerSideImpl connection = new ServerSideImpl(upload, download)) { connection.begin(); synchronized (connection) { - connection.wait(); + while (!connection.ready) { + connection.wait(); + } } PrintStream stdout = new PrintStream(connection.streamStdout(), false, connection.encoding.name()); PrintStream stderr = new PrintStream(connection.streamStderr(), true, connection.encoding.name()); -- GitLab