8042088: Sjavac spawns external processes in a unnecessarily complex and platform dependent way

Refactoring of the sjavac background fork code.

Reviewed-by: jfranck, ohrstrom
This commit is contained in:
Andreas Lundblad 2014-11-03 10:20:34 +01:00
parent 44213132ab
commit 78ab98e8fa
7 changed files with 188 additions and 181 deletions

View File

@ -91,7 +91,7 @@ public class CompileJavaPackages implements Transformer {
// Get maximum heap size from the server!
SysInfo sysinfo = sjavac.getSysInfo();
if (sysinfo.numCores == -1) {
if (sysinfo == null) {
Log.error("Could not query server for sysinfo!");
return false;
}

View File

@ -25,31 +25,33 @@
package com.sun.tools.sjavac.client;
import java.io.BufferedReader;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Scanner;
import java.util.Set;
import com.sun.tools.sjavac.Log;
import com.sun.tools.sjavac.ProblemException;
import com.sun.tools.sjavac.Util;
import com.sun.tools.sjavac.options.OptionHelper;
import com.sun.tools.sjavac.options.Options;
import com.sun.tools.sjavac.server.CompilationResult;
import com.sun.tools.sjavac.server.PortFile;
import com.sun.tools.sjavac.server.Sjavac;
import com.sun.tools.sjavac.server.SjavacServer;
import com.sun.tools.sjavac.server.SysInfo;
import com.sun.tools.sjavac.options.Options;
/**
* Sjavac implementation that delegates requests to a SjavacServer.
@ -64,10 +66,9 @@ public class SjavacClient implements Sjavac {
// The id can perhaps be used in the future by the javac server to reuse the
// JavaCompiler instance for several compiles using the same id.
private final String id;
private final String portfileName;
private final PortFile portFile;
private final String logfile;
private final String stdouterrfile;
private final boolean background;
// Default keepalive for server is 120 seconds.
// I.e. it will accept 120 seconds of inactivity before quitting.
@ -88,16 +89,27 @@ public class SjavacClient implements Sjavac {
// Store the server conf settings here.
private final String settings;
public SjavacClient(Options options) {
// This constructor should not throw FileNotFoundException (to be resolved
// in JDK-8060030)
public SjavacClient(Options options) throws FileNotFoundException {
String tmpServerConf = options.getServerConf();
String serverConf = (tmpServerConf!=null)? tmpServerConf : "";
String tmpId = Util.extractStringOption("id", serverConf);
id = (tmpId!=null) ? tmpId : "id"+(((new java.util.Random()).nextLong())&Long.MAX_VALUE);
String p = Util.extractStringOption("portfile", serverConf);
portfileName = (p!=null) ? p : options.getStateDir().toFile().getAbsolutePath()+File.separatorChar+"javac_server";
String defaultPortfile = options.getStateDir()
.resolve("javac_server")
.toAbsolutePath()
.toString();
String portfileName = Util.extractStringOption("portfile", serverConf, defaultPortfile);
try {
portFile = SjavacServer.getPortFile(portfileName);
} catch (FileNotFoundException e) {
// Reached for instance if directory of port file does not exist
Log.error("Port file inaccessable: " + e);
throw e;
}
logfile = Util.extractStringOption("logfile", serverConf, portfileName + ".javaclog");
stdouterrfile = Util.extractStringOption("stdouterrfile", serverConf, portfileName + ".stdouterr");
background = Util.extractBooleanOption("background", serverConf, true);
sjavacForkCmd = Util.extractStringOption("sjavac", serverConf, "sjavac");
int poolsize = Util.extractIntOption("poolsize", serverConf);
keepalive = Util.extractIntOption("keepalive", serverConf, 120);
@ -138,8 +150,11 @@ public class SjavacClient implements Sjavac {
return (SysInfo) ois.readObject();
} catch (IOException | ClassNotFoundException ex) {
Log.error("[CLIENT] Exception caught: " + ex);
StringWriter sw = new StringWriter();
ex.printStackTrace(new PrintWriter(sw));
Log.debug(Util.getStackTrace(ex));
} catch (InterruptedException ie) {
Thread.currentThread().interrupt(); // Restore interrupt
Log.error("[CLIENT] getSysInfo interrupted.");
Log.debug(Util.getStackTrace(ie));
}
return null;
}
@ -170,106 +185,127 @@ public class SjavacClient implements Sjavac {
oos.flush();
result = (CompilationResult) ois.readObject();
} catch (IOException | ClassNotFoundException ex) {
Log.error("Exception caught: " + ex);
Log.error("[CLIENT] Exception caught: " + ex);
result = new CompilationResult(CompilationResult.ERROR_FATAL);
result.stderr = Util.getStackTrace(ex);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt(); // Restore interrupt
Log.error("[CLIENT] compile interrupted.");
result = new CompilationResult(CompilationResult.ERROR_FATAL);
result.stderr = Util.getStackTrace(ie);
}
return result;
}
private Socket tryConnect() throws IOException {
PortFile portFile;
try {
// This should be taken care of at a higher level (JDK-8048451)
portFile = SjavacServer.getPortFile(portfileName);
} catch (FileNotFoundException e) {
// Reached for instance if directory of port file does not exist
Log.error("Port file inaccessable: " + e);
throw new RuntimeException(e);
}
for (int i = 0; i < MAX_CONNECT_ATTEMPTS; i++) {
Log.info(String.format("Trying to connect (attempt %d of %d)",
i+1, MAX_CONNECT_ATTEMPTS));
/*
* Makes MAX_CONNECT_ATTEMPTS attepmts to connect to server.
*/
private Socket tryConnect() throws IOException, InterruptedException {
makeSureServerIsRunning(portFile);
int attempt = 0;
while (true) {
Log.info("Trying to connect. Attempt " + (++attempt) + " of " + MAX_CONNECT_ATTEMPTS);
try {
if (!makeSureServerIsRunning(portFile))
continue;
Socket socket = new Socket();
InetAddress localhost = InetAddress.getByName(null);
socket.connect(new InetSocketAddress(localhost, portFile.getPort()),
CONNECTION_TIMEOUT);
return socket;
} catch (ProblemException | IOException ex) {
Log.error("Caught exception during tryConnect: " + ex);
}
try {
Thread.sleep(WAIT_BETWEEN_CONNECT_ATTEMPTS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
throw new IOException("Could not connect to server");
}
private boolean makeSureServerIsRunning(PortFile portFile)
throws IOException, ProblemException, FileNotFoundException {
synchronized (portFile) {
portFile.lock();
portFile.getValues();
portFile.unlock();
}
if (!portFile.containsPortInfo()) {
String forkCmd = SjavacServer.fork(sjavacForkCmd,
portFile.getFilename(),
logfile,
poolsize,
keepalive,
System.err,
stdouterrfile,
background);
if (!portFile.waitForValidValues()) {
// This can be simplified once JDK-8048457 has been addressed
// since we won't have an SjavacClient if background = false
if (background) {
// There seems be some problem with spawning the external
// process (for instance no fork command provided and no
// sjavac on path)
StringWriter sw = new StringWriter();
SjavacClient.printFailedAttempt(forkCmd,
stdouterrfile,
new PrintWriter(sw));
Log.error(sw.toString());
return makeConnectionAttempt();
} catch (IOException ex) {
Log.error("Connection attempt failed: " + ex.getMessage());
if (attempt >= MAX_CONNECT_ATTEMPTS) {
Log.error("Giving up");
throw new IOException("Could not connect to server", ex);
}
}
Thread.sleep(WAIT_BETWEEN_CONNECT_ATTEMPTS);
}
return portFile.containsPortInfo();
}
private Socket makeConnectionAttempt() throws IOException {
Socket socket = new Socket();
InetAddress localhost = InetAddress.getByName(null);
InetSocketAddress address = new InetSocketAddress(localhost, portFile.getPort());
socket.connect(address, CONNECTION_TIMEOUT);
Log.info("Connected");
return socket;
}
public static void printFailedAttempt(String cmd, String f, PrintWriter err) {
err.println("---- Failed to start javac server with this command -----");
err.println(cmd);
try {
BufferedReader in = new BufferedReader(new FileReader(f));
err.println("---- stdout/stderr output from attempt to start javac server -----");
for (;;) {
String l = in.readLine();
if (l == null) {
break;
}
err.println(l);
}
err.println("------------------------------------------------------------------");
} catch (Exception e) {
err.println("The stdout/stderr output in file " + f + " does not exist and the server did not start.");
/*
* Will return immediately if a server already seems to be running,
* otherwise fork a new server and block until it seems to be running.
*/
private void makeSureServerIsRunning(PortFile portFile)
throws IOException, InterruptedException {
portFile.lock();
portFile.getValues();
portFile.unlock();
if (portFile.containsPortInfo()) {
// Server seems to already be running
return;
}
// Fork a new server and wait for it to start
SjavacClient.fork(sjavacForkCmd,
portFile,
logfile,
poolsize,
keepalive,
System.err,
stdouterrfile);
}
@Override
public void shutdown() {
// Nothing to clean up
}
/*
* Fork a server process process and wait for server to come around
*/
public static void fork(String sjavacCmd,
PortFile portFile,
String logfile,
int poolsize,
int keepalive,
final PrintStream err,
String stdouterrfile)
throws IOException, InterruptedException {
List<String> cmd = new ArrayList<>();
cmd.addAll(Arrays.asList(OptionHelper.unescapeCmdArg(sjavacCmd).split(" ")));
cmd.add("--startserver:"
+ "portfile=" + portFile.getFilename()
+ ",logfile=" + logfile
+ ",stdouterrfile=" + stdouterrfile
+ ",poolsize=" + poolsize
+ ",keepalive="+ keepalive);
Process p = null;
Log.info("Starting server. Command: " + String.join(" ", cmd));
try {
// If the cmd for some reason can't be executed (file not found, or
// is not executable) this will throw an IOException with a decent
// error message.
p = new ProcessBuilder(cmd)
.redirectErrorStream(true)
.redirectOutput(new File(stdouterrfile))
.start();
// Throws an IOException if no valid values materialize
portFile.waitForValidValues();
} catch (IOException ex) {
// Log and rethrow exception
Log.error("Faild to launch server.");
Log.error(" Message: " + ex.getMessage());
String rc = p == null || p.isAlive() ? "n/a" : "" + p.exitValue();
Log.error(" Server process exit code: " + rc);
Log.error("Server log:");
Log.error("------- Server log start -------");
try (Scanner s = new Scanner(new File(stdouterrfile))) {
while (s.hasNextLine())
Log.error(s.nextLine());
}
Log.error("------- Server log end ---------");
throw ex;
}
}
}

View File

@ -25,7 +25,6 @@
package com.sun.tools.sjavac.options;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
@ -159,4 +158,9 @@ public abstract class OptionHelper {
}
}
}
public static String unescapeCmdArg(String arg) {
return arg.replaceAll("%20", " ")
.replaceAll("%2C", ",");
}
}

View File

@ -33,6 +33,8 @@ import java.nio.channels.ClosedChannelException;
import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.channels.FileLockInterruptionException;
import java.util.concurrent.Semaphore;
import com.sun.tools.javac.util.Assert;
import com.sun.tools.sjavac.Log;
@ -61,7 +63,12 @@ public class PortFile {
private File stopFile;
private RandomAccessFile rwfile;
private FileChannel channel;
// FileLock used to solve inter JVM synchronization, lockSem used to avoid
// JVM internal OverlappingFileLockExceptions.
// Class invariant: lock.isValid() <-> lockSem.availablePermits() == 0
private FileLock lock;
private Semaphore lockSem = new Semaphore(1);
private boolean containsPortInfo;
private int serverPort;
@ -88,7 +95,8 @@ public class PortFile {
/**
* Lock the port file.
*/
public void lock() throws IOException {
public void lock() throws IOException, InterruptedException {
lockSem.acquire();
lock = channel.lock();
}
@ -195,34 +203,37 @@ public class PortFile {
Assert.check(lock != null);
lock.release();
lock = null;
lockSem.release();
}
/**
* Wait for the port file to contain values that look valid.
* Return true, if a-ok, false if the valid values did not materialize within 5 seconds.
*/
public synchronized boolean waitForValidValues() throws IOException, FileNotFoundException {
for (int tries = 0; tries < 50; tries++) {
public void waitForValidValues() throws IOException, InterruptedException {
final int MAX_ATTEMPTS = 10;
final int MS_BETWEEN_ATTEMPTS = 500;
long startTime = System.currentTimeMillis();
for (int attempt = 0; ; attempt++) {
Log.debug("Looking for valid port file values...");
lock();
getValues();
unlock();
if (containsPortInfo) {
Log.debug("Found valid values in port file after waiting "+(tries*100)+"ms");
return true;
Log.debug("Valid port file values found after " + (System.currentTimeMillis() - startTime) + " ms");
return;
}
try {
Thread.sleep(100);
} catch (InterruptedException e)
{}
if (attempt >= MAX_ATTEMPTS) {
throw new IOException("No port file values materialized. Giving up after " +
(System.currentTimeMillis() - startTime) + " ms");
}
Thread.sleep(MS_BETWEEN_ATTEMPTS);
}
Log.debug("Gave up waiting for valid values in port file");
return false;
}
/**
* Check if the portfile still contains my values, assuming that I am the server.
*/
public synchronized boolean stillMyValues() throws IOException, FileNotFoundException {
public boolean stillMyValues() throws IOException, FileNotFoundException, InterruptedException {
for (;;) {
try {
lock();

View File

@ -75,6 +75,10 @@ public class PortFileMonitor {
} catch (IOException e) {
e.printStackTrace(server.theLog);
server.flushLog();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
e.printStackTrace(server.theLog);
server.flushLog();
}
}
};

View File

@ -46,8 +46,8 @@ public class ServerMain {
try {
SjavacServer server = new SjavacServer(args[0], System.err);
exitCode = server.startServer();
} catch (IOException ioex) {
ioex.printStackTrace();
} catch (IOException | InterruptedException ex) {
ex.printStackTrace();
exitCode = -1;
}

View File

@ -24,7 +24,6 @@
*/
package com.sun.tools.sjavac.server;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.PrintStream;
@ -34,16 +33,14 @@ import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.atomic.AtomicBoolean;
import com.sun.tools.sjavac.ProblemException;
import com.sun.tools.sjavac.Util;
import com.sun.tools.sjavac.comp.SjavacImpl;
import com.sun.tools.sjavac.comp.PooledSjavac;
import com.sun.tools.sjavac.comp.SjavacImpl;
/**
* The JavacServer class contains methods both to setup a server that responds to requests and methods to connect to this server.
@ -95,13 +92,26 @@ public class SjavacServer implements Terminable {
private static Map<String, Long> maxServerMemory;
public SjavacServer(String settings, PrintStream err) throws FileNotFoundException {
// Extract options. TODO: Change to proper constructor args
portfilename = Util.extractStringOption("portfile", settings);
logfile = Util.extractStringOption("logfile", settings);
stdouterrfile = Util.extractStringOption("stdouterrfile", settings);
keepalive = Util.extractIntOption("keepalive", settings, 120);
poolsize = Util.extractIntOption("poolsize", settings,
Runtime.getRuntime().availableProcessors());
this(Util.extractStringOption("portfile", settings),
Util.extractStringOption("logfile", settings),
Util.extractStringOption("stdouterrfile", settings),
Util.extractIntOption("poolsize", settings, Runtime.getRuntime().availableProcessors()),
Util.extractIntOption("keepalive", settings, 120),
err);
}
public SjavacServer(String portfilename,
String logfile,
String stdouterrfile,
int poolsize,
int keepalive,
PrintStream err)
throws FileNotFoundException {
this.portfilename = portfilename;
this.logfile = logfile;
this.stdouterrfile = stdouterrfile;
this.poolsize = poolsize;
this.keepalive = keepalive;
this.err = err;
myCookie = new Random().nextLong();
@ -180,7 +190,7 @@ public class SjavacServer implements Terminable {
* Start a server using a settings string. Typically: "--startserver:portfile=/tmp/myserver,poolsize=3" and the string "portfile=/tmp/myserver,poolsize=3"
* is sent as the settings parameter. Returns 0 on success, -1 on failure.
*/
public int startServer() throws IOException {
public int startServer() throws IOException, InterruptedException {
long serverStart = System.currentTimeMillis();
// The port file is locked and the server port and cookie is written into it.
@ -250,64 +260,6 @@ public class SjavacServer implements Terminable {
return 0;
}
/**
* Fork a background process. Returns the command line used that can be printed if something failed.
*/
public static String fork(String sjavac, String portfile, String logfile, int poolsize, int keepalive,
final PrintStream err, String stdouterrfile, boolean background)
throws IOException, ProblemException {
if (stdouterrfile != null && stdouterrfile.trim().equals("")) {
stdouterrfile = null;
}
final String startserver = "--startserver:portfile=" + portfile + ",logfile=" + logfile + ",stdouterrfile=" + stdouterrfile + ",poolsize=" + poolsize + ",keepalive="+ keepalive;
if (background) {
sjavac += "%20" + startserver;
sjavac = sjavac.replaceAll("%20", " ");
sjavac = sjavac.replaceAll("%2C", ",");
// If the java/sh/cmd launcher fails the failure will be captured by stdouterr because of the redirection here.
String[] cmd = {"/bin/sh", "-c", sjavac + " >> " + stdouterrfile + " 2>&1"};
if (!(new File("/bin/sh")).canExecute()) {
ArrayList<String> wincmd = new ArrayList<>();
wincmd.add("cmd");
wincmd.add("/c");
wincmd.add("start");
wincmd.add("cmd");
wincmd.add("/c");
wincmd.add(sjavac + " >> " + stdouterrfile + " 2>&1");
cmd = wincmd.toArray(new String[wincmd.size()]);
}
Process pp = null;
try {
pp = Runtime.getRuntime().exec(cmd);
} catch (Exception e) {
e.printStackTrace(err);
e.printStackTrace(new PrintWriter(stdouterrfile));
}
StringBuilder rs = new StringBuilder();
for (String s : cmd) {
rs.append(s + " ");
}
return rs.toString();
}
// Do not spawn a background server, instead run it within the same JVM.
Thread t = new Thread() {
@Override
public void run() {
try {
SjavacServer server = new SjavacServer(startserver, err);
server.startServer();
} catch (Throwable t) {
t.printStackTrace(err);
}
}
};
t.setDaemon(true);
t.start();
return "";
}
@Override
public void shutdown(String quitMsg) {
if (!keepAcceptingRequests.compareAndSet(true, false)) {