Fixing checkstyle test hang with nc version >= 1.110
netcat (nc) 1.110 changed the behavior of how sockets are closed when
stdin is at EOF or closed. Previously, nc would call shutdown (TCP FIN
on the write side of the socket) when the end of stdin was reached.
Now, shutdown is only called if '-N' is passed as an argument.
This change was introduced into Ubuntu's fork of OpenBSD nc in Nov. 2016.
So, the affected versions were Ubuntu 17.04+ as well as any other
distribution that uses nc >= 1.110.
This change of behavior causes the call to ByteStreams.toByteArray()
to hang indefinietly, and thus checkstyle tests to hang indefinitely.
Rather than try to figure out which version of nc is present and set
the -N option, we will use an empty line as a sentinel and stop parsing
input when the first empty line is encountered. For this, we need two
changes: (1) send a newline when checking the socket in start-buck-daemon
and (2) send a newline at the end of the file list in onos.bucklet
We also set SO_TIMEOUT to 1 second and will return an exception if
the socket times out. This will prevent tests from hanging indefinitely.
Change-Id: If46b4b78ae89312e1afa0563f63100ae67762f0a
diff --git a/bucklets/onos.bucklet b/bucklets/onos.bucklet
index 8f5a11b..d863db1 100644
--- a/bucklets/onos.bucklet
+++ b/bucklets/onos.bucklet
@@ -23,7 +23,8 @@
if srcs:
base = get_base_path()
- files = '%s\n%s\n' % (name, base) + '\n'.join(['%s/%s' % (base, s) for s in srcs])
+ # module name; base filepath; files (line-separated); empty line (to signify end of stream)
+ files = '%s\n%s\n' % (name, base) + '\n'.join(['%s/%s' % (base, s) for s in srcs]) + '\n\n'
genrule(
name = name + '-checkstyle-files',
@@ -45,6 +46,7 @@
'$(location //tools/build/conf:checkstyle-xml)',
'$(location //tools/build/conf:suppressions-xml)',
],
+ #TODO test_rule_timeout_ms seems to be ignored on Linux
test_rule_timeout_ms = 45000,
labels = [ 'checkstyle' ],
)
diff --git a/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckDaemon.java b/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckDaemon.java
index 9fdfbf2..56deadd 100644
--- a/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckDaemon.java
+++ b/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckDaemon.java
@@ -16,12 +16,13 @@
package org.onosproject.buckdaemon;
-import com.google.common.io.ByteStreams;
import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
import org.onosproject.checkstyle.CheckstyleRunner;
import java.io.IOException;
import java.io.PrintStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.net.ServerSocket;
import java.net.Socket;
import java.nio.ByteBuffer;
@@ -34,11 +35,11 @@
import java.util.Map;
import java.util.Timer;
import java.util.TimerTask;
-import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
-import static java.nio.file.StandardOpenOption.*;
+import static java.nio.file.StandardOpenOption.CREATE;
+import static java.nio.file.StandardOpenOption.WRITE;
/**
* Buck daemon process.
@@ -95,7 +96,7 @@
Process p = Runtime.getRuntime().exec(cmd);
p.waitFor();
if (p.exitValue() != 0) {
- System.err.println("shutting down...");
+ debug("shutting down...");
System.exit(0);
}
} catch (IOException | InterruptedException e) {
@@ -115,7 +116,7 @@
FileChannel channel = FileChannel.open(portLockPath, WRITE, CREATE);
FileLock lock = channel.tryLock();
if (lock == null) {
- System.out.println("Server is already running");
+ debug("Server is already running");
System.exit(1);
} //else, hold the lock until the JVM exits
@@ -130,7 +131,7 @@
try {
channel.truncate(0);
channel.close();
- System.err.println("tear down...");
+ debug("tear down...");
Files.delete(portLockPath);
} catch (IOException e) {
//no-op: shutting down
@@ -142,6 +143,7 @@
int port = server.getLocalPort();
channel.truncate(0);
channel.write(ByteBuffer.wrap(Integer.toString(port).getBytes()));
+ channel.force(false); // flush the port number to disk
// Instantiate a Checkstyle runner and executor; serve until exit...
ExecutorService executor = Executors.newCachedThreadPool();
@@ -171,26 +173,34 @@
@Override
public void run() {
try {
- BuckTaskContext context = new BuckTaskContext(socket.getInputStream());
- String taskName = context.taskName();
- if (!taskName.isEmpty()) {
+ try {
+ socket.setSoTimeout(1_000); //reads should time out after 1 second
+ BuckTaskContext context = new BuckTaskContext(socket.getInputStream());
+
+ String taskName = context.taskName();
BuckTask task = tasks.get(taskName);
if (task != null) {
- System.out.println(String.format("Executing task '%s'", taskName));
+ debug(String.format("Executing task '%s'", taskName));
try {
task.execute(context);
for (String line : context.output()) {
- output(socket, line);
+ send(socket, line);
}
- // TODO should we catch Exception, RuntimeException, or something specific?
+ // TODO should we catch Exception, RuntimeException, or something specific?
} catch (Throwable e) {
e.printStackTrace(new PrintStream(socket.getOutputStream()));
}
} else {
String message = String.format("No task named '%s'", taskName);
- System.out.print(message);
- output(socket, message);
+ debug(message);
+ send(socket, message);
}
+ } catch (Throwable e) {
+ StringWriter writer = new StringWriter();
+ e.printStackTrace(new PrintWriter(writer));
+ String stacktrace = writer.toString();
+ debug(stacktrace);
+ send(socket, stacktrace);
}
socket.getOutputStream().flush();
socket.close();
@@ -199,8 +209,13 @@
}
}
- private void output(Socket socket, String line) throws IOException {
- socket.getOutputStream().write((line + "\n").getBytes());
- }
+ }
+
+ private static void send(Socket socket, String line) throws IOException {
+ socket.getOutputStream().write((line + "\n").getBytes());
+ }
+
+ private static void debug(String message) {
+ // no-op; print to System.out if needed
}
}
diff --git a/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckTaskContext.java b/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckTaskContext.java
index 7e2046f..b8d189c 100644
--- a/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckTaskContext.java
+++ b/tools/build/conf/src/main/java/org/onosproject/buckdaemon/BuckTaskContext.java
@@ -17,11 +17,12 @@
package org.onosproject.buckdaemon;
import com.google.common.collect.ImmutableList;
-import com.google.common.io.ByteStreams;
+import com.google.common.collect.Lists;
+import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
-import java.util.ArrayList;
+import java.io.InputStreamReader;
import java.util.List;
import static com.google.common.base.Preconditions.checkArgument;
@@ -33,17 +34,36 @@
private final String taskName;
private final ImmutableList<String> input;
- private final List<String> output = new ArrayList<>();
+ private final List<String> output;
- BuckTaskContext(InputStream inputString) throws IOException {
- String[] split = new String(ByteStreams.toByteArray(inputString)).split("\n");
- checkArgument(split.length >= 1, "Request must contain at least task type");
- this.taskName = split[0];
- ImmutableList.Builder<String> builder = ImmutableList.builder();
- for (int i = 1; i < split.length; i++) {
- builder.add(split[i]);
+ BuckTaskContext(InputStream inputStream) throws IOException {
+ ImmutableList<String> lines = slurpInput(inputStream);
+ checkArgument(lines.size() >= 1 && !lines.get(0).isEmpty(),
+ "Request must contain at least task type");
+ this.taskName = lines.get(0);
+ this.input = lines.subList(1, lines.size());
+ this.output = Lists.newArrayList();
+ }
+
+ /**
+ * Reads all input, line by line, from a stream until an empty line or EOF is encountered.
+ *
+ * @param stream input stream
+ * @return the lines of the input
+ * @throws IOException
+ */
+ private static ImmutableList<String> slurpInput(InputStream stream) throws IOException {
+ ImmutableList.Builder<String> lines = ImmutableList.builder();
+ BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(stream));
+ while(true) {
+ String line = bufferedReader.readLine();
+ if (line == null || line.trim().length() == 0) {
+ // Empty line or EOF
+ break;
+ }
+ lines.add(line);
}
- input = builder.build();
+ return lines.build();
}
/**
diff --git a/tools/build/conf/start-buck-daemon b/tools/build/conf/start-buck-daemon
index e6e9c1e..b747376 100755
--- a/tools/build/conf/start-buck-daemon
+++ b/tools/build/conf/start-buck-daemon
@@ -41,7 +41,7 @@
}
function check_socket() {
- nc localhost $(port) < /dev/null 2>/dev/null
+ printf "\n" | nc localhost $(port) 2>/dev/null
return $?
}