Merge branch '3-4-10-wrapper-states' into 'master'

Add more wrapper states, allow wrapper to be reused

Closes #10, #4, and #3

See merge request !9

onionwrapper-android/src/main/java/org/briarproject/onionwrapper/AndroidTorWrapper.java

@@ -5,7 +5,6 @@ import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.res.Resources;
-import android.os.Build;
 
 import org.briarproject.android.dontkillmelib.wakelock.AndroidWakeLock;
 import org.briarproject.android.dontkillmelib.wakelock.AndroidWakeLockManager;
@@ -24,6 +23,9 @@ import java.util.logging.Logger;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
+import static android.os.Build.CPU_ABI;
+import static android.os.Build.CPU_ABI2;
+import static android.os.Build.SUPPORTED_ABIS;
 import static android.os.Build.VERSION.SDK_INT;
 import static java.util.Arrays.asList;
 import static java.util.logging.Level.INFO;
@@ -42,8 +44,7 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 	private static final String OBFS4_LIB_NAME = "libobfs4proxy.so";
 	private static final String SNOWFLAKE_LIB_NAME = "libsnowflake.so";
 
-	private static final Logger LOG =
-			getLogger(AndroidTorWrapper.class.getName());
+	private static final Logger LOG = getLogger(AndroidTorWrapper.class.getName());
 
 	private final Application app;
 	private final AndroidWakeLock wakeLock;
@@ -74,8 +75,7 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 			File torDirectory,
 			int torSocksPort,
 			int torControlPort) {
-		super(ioExecutor, eventExecutor, architecture, torDirectory,
-				torSocksPort, torControlPort);
+		super(ioExecutor, eventExecutor, architecture, torDirectory, torSocksPort, torControlPort);
 		this.app = app;
 		wakeLock = wakeLockManager.createWakeLock("TorPlugin");
 		String nativeLibDir = app.getApplicationInfo().nativeLibraryDir;
@@ -119,7 +119,7 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 	}
 
 	@Override
-	public void stop() throws IOException {
+	public void stop() throws IOException, InterruptedException {
 		try {
 			super.stop();
 		} finally {
@@ -139,8 +139,7 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 
 	@Override
 	protected File getSnowflakeExecutableFile() {
-		return snowflakeLib.exists() ? snowflakeLib
-				: super.getSnowflakeExecutableFile();
+		return snowflakeLib.exists() ? snowflakeLib : super.getSnowflakeExecutableFile();
 	}
 
 	@Override
@@ -150,18 +149,15 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 
 	@Override
 	protected void installObfs4Executable() throws IOException {
-		installExecutable(super.getObfs4ExecutableFile(), obfs4Lib,
-				OBFS4_LIB_NAME);
+		installExecutable(super.getObfs4ExecutableFile(), obfs4Lib, OBFS4_LIB_NAME);
 	}
 
 	@Override
 	protected void installSnowflakeExecutable() throws IOException {
-		installExecutable(super.getSnowflakeExecutableFile(), snowflakeLib,
-				SNOWFLAKE_LIB_NAME);
+		installExecutable(super.getSnowflakeExecutableFile(), snowflakeLib, SNOWFLAKE_LIB_NAME);
 	}
 
-	private void installExecutable(File extracted, File lib, String libName)
-			throws IOException {
+	private void installExecutable(File extracted, File lib, String libName) throws IOException {
 		if (lib.exists()) {
 			// If an older version left behind a binary, delete it
 			if (extracted.exists()) {
@@ -177,8 +173,7 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 		}
 	}
 
-	private void extractLibraryFromApk(String libName, File dest)
-			throws IOException {
+	private void extractLibraryFromApk(String libName, File dest) throws IOException {
 		File sourceDir = new File(app.getApplicationInfo().sourceDir);
 		if (sourceDir.isFile()) {
 			// Look for other APK files in the same directory, if we're allowed
@@ -189,12 +184,10 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 		for (File apk : findApkFiles(sourceDir)) {
 			@SuppressWarnings("IOStreamConstructor")
 			ZipInputStream zin = new ZipInputStream(new FileInputStream(apk));
-			for (ZipEntry e = zin.getNextEntry(); e != null;
-			     e = zin.getNextEntry()) {
+			for (ZipEntry e = zin.getNextEntry(); e != null; e = zin.getNextEntry()) {
 				if (libPaths.contains(e.getName())) {
 					if (LOG.isLoggable(INFO)) {
-						LOG.info("Extracting " + e.getName()
-								+ " from " + apk.getAbsolutePath());
+						LOG.info("Extracting " + e.getName() + " from " + apk.getAbsolutePath());
 					}
 					extract(zin, dest); // Zip input stream will be closed
 					return;
@@ -243,10 +236,10 @@ public class AndroidTorWrapper extends AbstractTorWrapper {
 	private Collection<String> getSupportedArchitectures() {
 		List<String> abis = new ArrayList<>();
 		if (SDK_INT >= 21) {
-			abis.addAll(asList(Build.SUPPORTED_ABIS));
+			abis.addAll(asList(SUPPORTED_ABIS));
 		} else {
-			abis.add(Build.CPU_ABI);
-			if (Build.CPU_ABI2 != null) abis.add(Build.CPU_ABI2);
+			abis.add(CPU_ABI);
+			if (CPU_ABI2 != null) abis.add(CPU_ABI2);
 		}
 		return abis;
 	}

onionwrapper-core/src/main/java/org/briarproject/onionwrapper/AbstractTorWrapper.java

@@ -19,8 +19,9 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Scanner;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Executor;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -44,7 +45,11 @@ import static org.briarproject.onionwrapper.TorUtils.tryToClose;
 import static org.briarproject.onionwrapper.TorWrapper.TorState.CONNECTED;
 import static org.briarproject.onionwrapper.TorWrapper.TorState.CONNECTING;
 import static org.briarproject.onionwrapper.TorWrapper.TorState.DISABLED;
-import static org.briarproject.onionwrapper.TorWrapper.TorState.STARTING_STOPPING;
+import static org.briarproject.onionwrapper.TorWrapper.TorState.NOT_STARTED;
+import static org.briarproject.onionwrapper.TorWrapper.TorState.STARTED;
+import static org.briarproject.onionwrapper.TorWrapper.TorState.STARTING;
+import static org.briarproject.onionwrapper.TorWrapper.TorState.STOPPED;
+import static org.briarproject.onionwrapper.TorWrapper.TorState.STOPPING;
 
 @InterfaceNotNullByDefault
 abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
@@ -63,8 +68,7 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 	private static final String OWNER = "__OwningControllerProcess";
 	private static final int COOKIE_TIMEOUT_MS = 3000;
 	private static final int COOKIE_POLLING_INTERVAL_MS = 200;
-	private static final Pattern BOOTSTRAP_PERCENTAGE =
-			Pattern.compile(".*PROGRESS=(\\d{1,3}).*");
+	private static final Pattern BOOTSTRAP_PERCENTAGE = Pattern.compile(".*PROGRESS=(\\d{1,3}).*");
 
 	protected final Executor ioExecutor;
 	protected final Executor eventExecutor;
@@ -72,10 +76,10 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 	private final File torDirectory, configFile, doneFile, cookieFile;
 	private final int torSocksPort;
 	private final int torControlPort;
-	private final AtomicBoolean used = new AtomicBoolean(false);
 
 	protected final NetworkState state = new NetworkState();
 
+	private volatile Process torProcess = null;
 	private volatile Socket controlSocket = null;
 	private volatile TorControlConnection controlConnection = null;
 
@@ -83,8 +87,7 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 
 	protected abstract long getLastUpdateTime();
 
-	protected abstract InputStream getResourceInputStream(String name,
-			String extension);
+	protected abstract InputStream getResourceInputStream(String name, String extension);
 
 	AbstractTorWrapper(Executor ioExecutor,
 			Executor eventExecutor,
@@ -123,7 +126,7 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 
 	@Override
 	public void start() throws IOException, InterruptedException {
-		if (used.getAndSet(true)) throw new IllegalStateException();
+		state.setStarting();
 		if (!torDirectory.exists()) {
 			if (!torDirectory.mkdirs()) {
 				throw new IOException("Could not create Tor directory");
@@ -133,17 +136,16 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 		if (!assetsAreUpToDate()) installAssets();
 		// Start from the default config every time
 		extract(getConfigInputStream(), configFile);
-		if (cookieFile.exists() && !cookieFile.delete())
+		if (cookieFile.exists() && !cookieFile.delete()) {
 			LOG.warning("Old auth cookie not deleted");
+		}
 		// Start a new Tor process
 		LOG.info("Starting Tor");
 		File torFile = getTorExecutableFile();
 		String torPath = torFile.getAbsolutePath();
 		String configPath = configFile.getAbsolutePath();
 		String pid = String.valueOf(getProcessId());
-		Process torProcess;
-		ProcessBuilder pb =
-				new ProcessBuilder(torPath, "-f", configPath, OWNER, pid);
+		ProcessBuilder pb = new ProcessBuilder(torPath, "-f", configPath, OWNER, pid);
 		Map<String, String> env = pb.environment();
 		env.put("HOME", torDirectory.getAbsolutePath());
 		pb.directory(torDirectory);
@@ -154,7 +156,7 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 			throw new IOException(e);
 		}
 		// Wait for the Tor process to start
-		waitForTorToStart(torProcess);
+		waitForTorToStart(requireNonNull(torProcess));
 		// Wait for the auth cookie file to be created/updated
 		long start = System.currentTimeMillis();
 		while (cookieFile.length() < 32) {
@@ -243,8 +245,7 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 
 	private InputStream getExecutableInputStream(String basename) {
 		String ext = getExecutableExtension();
-		return requireNonNull(
-				getResourceInputStream(architecture + "/" + basename, ext));
+		return requireNonNull(getResourceInputStream(architecture + "/" + basename, ext));
 	}
 
 	protected String getExecutableExtension() {
@@ -265,7 +266,6 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 		append(strb, "CookieAuthentication", 1);
 		append(strb, "DataDirectory", dataDirectory.getAbsolutePath());
 		append(strb, "DisableNetwork", 1);
-		append(strb, "RunAsDaemon", 1);
 		append(strb, "SafeSocks", 1);
 		append(strb, "SocksPort", torSocksPort);
 		strb.append("GeoIPFile\n");
@@ -295,29 +295,48 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 		}
 	}
 
-	protected void waitForTorToStart(Process torProcess)
-			throws InterruptedException, IOException {
-		Scanner stdout = new Scanner(torProcess.getInputStream());
-		// Log the first line of stdout (contains Tor and library versions)
-		if (stdout.hasNextLine()) LOG.info(stdout.nextLine());
-		// Read the process's stdout (and redirected stderr) until it detaches
-		while (stdout.hasNextLine()) stdout.nextLine();
-		stdout.close();
-		// Wait for the process to detach or exit
-		int exit = torProcess.waitFor();
-		if (exit != 0) throw new IOException("Tor exited with value " + exit);
+	protected void waitForTorToStart(Process torProcess) throws InterruptedException, IOException {
+		// Wait for the control port to be opened, then continue to read Tor's
+		// stdout and stderr in a background thread until it exits.
+		BlockingQueue<Boolean> success = new ArrayBlockingQueue<>(1);
+		ioExecutor.execute(() -> {
+			boolean started = false;
+			// Read the process's stdout (and redirected stderr)
+			Scanner stdout = new Scanner(torProcess.getInputStream());
+			// Log the first line of stdout (contains Tor and library versions)
+			if (stdout.hasNextLine()) LOG.info(stdout.nextLine());
+			// Startup has succeeded when the control port is open
+			while (stdout.hasNextLine()) {
+				String line = stdout.nextLine();
+				if (!started && line.contains("Opened Control listener")) {
+					success.add(true);
+					started = true;
+				}
+			}
+			stdout.close();
+			// If the control port wasn't opened, startup has failed
+			if (!started) success.add(false);
+			// Wait for the process to exit
+			try {
+				int exit = torProcess.waitFor();
+				if (LOG.isLoggable(INFO)) LOG.info("Tor exited with value " + exit);
+			} catch (InterruptedException e1) {
+				LOG.warning("Interrupted while waiting for Tor to exit");
+				Thread.currentThread().interrupt();
+			}
+		});
+		// Wait for the startup result
+		if (!success.take()) throw new IOException();
 	}
 
 	@Override
 	public HiddenServiceProperties publishHiddenService(int localPort,
 			int remotePort, @Nullable String privKey) throws IOException {
-		Map<Integer, String> portLines =
-				singletonMap(remotePort, "127.0.0.1:" + localPort);
+		Map<Integer, String> portLines = singletonMap(remotePort, "127.0.0.1:" + localPort);
 		// Use the control connection to set up the hidden service
 		Map<String, String> response;
 		if (privKey == null) {
-			response = getControlConnection().addOnion("NEW:ED25519-V3",
-					portLines, null);
+			response = getControlConnection().addOnion("NEW:ED25519-V3", portLines, null);
 		} else {
 			response = getControlConnection().addOnion(privKey, portLines);
 		}
@@ -362,14 +381,23 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 	}
 
 	@Override
-	public void stop() throws IOException {
-		state.setStopped();
-		if (controlSocket != null && controlConnection != null) {
-			LOG.info("Stopping Tor");
-			try {
+	public void stop() throws IOException, InterruptedException {
+		state.setStopping();
+		try {
+			if (controlConnection != null) {
 				controlConnection.shutdownTor("TERM");
+			}
+		} finally {
+			controlConnection = null;
+			tryToClose(controlSocket, LOG, WARNING);
+			controlSocket = null;
+			try {
+				if (torProcess != null) {
+					torProcess.waitFor();
+				}
 			} finally {
-				tryToClose(controlSocket, LOG, WARNING);
+				torProcess = null;
+				state.setStopped();
 			}
 		}
 	}
@@ -536,6 +564,10 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 		return controlConnection;
 	}
 
+	private enum ProcessState {
+		NOT_STARTED, STARTING, STARTED, STOPPING, STOPPED
+	}
+
 	@ThreadSafe
 	@NotNullByDefault
 	private class NetworkState {
@@ -545,9 +577,10 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 		private Observer observer = null;
 
 		@GuardedBy("this")
-		private boolean started = false,
-				stopped = false,
-				networkInitialised = false,
+		private ProcessState processState = ProcessState.NOT_STARTED;
+
+		@GuardedBy("this")
+		private boolean networkInitialised = false,
 				networkEnabled = false,
 				paddingEnabled = false,
 				ipv6Enabled = false,
@@ -566,8 +599,7 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 		@Nullable
 		private TorState state = null;
 
-		private synchronized void setObserver(
-				@Nullable Observer observer) {
+		private synchronized void setObserver(@Nullable Observer observer) {
 			this.observer = observer;
 		}
 
@@ -583,18 +615,50 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 			}
 		}
 
+		private synchronized void setStarting() {
+			// It's legal to call start() if the wrapper has never been started, or has been
+			// started and then stopped
+			if (processState != ProcessState.NOT_STARTED && processState != ProcessState.STOPPED) {
+				throw new IllegalStateException();
+			}
+			processState = ProcessState.STARTING;
+			updateState();
+		}
+
 		private synchronized void setStarted() {
-			started = true;
+			// It's illegal to call start() and stop() concurrently
+			if (processState != ProcessState.STARTING) throw new IllegalStateException();
+			processState = ProcessState.STARTED;
 			updateState();
 		}
 
 		@SuppressWarnings("BooleanMethodIsAlwaysInverted")
 		private synchronized boolean isTorRunning() {
-			return started && !stopped;
+			return processState == ProcessState.STARTED;
+		}
+
+		private synchronized void setStopping() {
+			// It's legal to call stop() if start() has returned or thrown an exception
+			if (processState != ProcessState.STARTING && processState != ProcessState.STARTED) {
+				throw new IllegalStateException();
+			}
+			processState = ProcessState.STOPPING;
+			updateState();
 		}
 
 		private synchronized void setStopped() {
-			stopped = true;
+			// It's illegal to call start() and stop() concurrently
+			if (processState != ProcessState.STOPPING) throw new IllegalStateException();
+			processState = ProcessState.STOPPED;
+			// Reset all state related to the process that has stopped
+			networkInitialised = false;
+			networkEnabled = false;
+			paddingEnabled = false;
+			ipv6Enabled = false;
+			circuitBuilt = false;
+			bootstrapPercentage = 0;
+			bridges = emptyList();
+			orConnectionsConnected = 0;
 			updateState();
 		}
 
@@ -668,8 +732,11 @@ abstract class AbstractTorWrapper implements EventHandler, TorWrapper {
 		}
 
 		private synchronized TorState getState() {
-			if (!started || stopped) return STARTING_STOPPING;
-			if (!networkInitialised) return CONNECTING;
+			if (processState == ProcessState.NOT_STARTED) return NOT_STARTED;
+			if (processState == ProcessState.STARTING) return STARTING;
+			if (processState == ProcessState.STOPPING) return STOPPING;
+			if (processState == ProcessState.STOPPED) return STOPPED;
+			if (!networkInitialised) return STARTED;
 			if (!networkEnabled) return DISABLED;
 			return bootstrapPercentage == 100 && circuitBuilt
 					&& orConnectionsConnected > 0 ? CONNECTED : CONNECTING;

onionwrapper-core/src/main/java/org/briarproject/onionwrapper/TorWrapper.java

@@ -17,19 +17,33 @@ public interface TorWrapper {
 	Logger LOG = getLogger(TorWrapper.class.getName());
 
 	/**
-	 * Starts the Tor process, but does not yet connect to the Tor Network.
+	 * Starts the Tor process, but does not yet connect to the Tor network.
 	 * Call {@link #enableNetwork(boolean)} for this.
 	 * <p>
-	 * This method must only be called once. To restart the Tor process, stop
-	 * this wrapper instance and then create a new instance.
+	 * This method waits for the Tor process to start before returning. Methods
+	 * that modify the wrapper's configuration
+	 * ({@link #publishHiddenService(int, int, String)},
+	 * {@link #removeHiddenService(String)}, {@link #enableNetwork(boolean)},
+	 * {@link #enableBridges(List)}, {@link #enableConnectionPadding(boolean)},
+	 * {@link #enableIpv6(boolean)}) should be called after this method returns.
+	 * <p>
+	 * Do not call this method concurrently with {@link #stop()}.
+	 * <p>
+	 * If this method throws an exception, call {@link #stop()} before trying
+	 * to call this method again.
 	 */
 	void start() throws IOException, InterruptedException;
 
 	/**
-	 * Tell the Tor process to stop and returns without waiting for the
-	 * process to exit.
+	 * Tell the Tor process to stop and waits for it to stop before returning.
+	 * <p>
+	 * The wrapper's configuration is reset, so if the wrapper is reused by
+	 * calling {@link #start()} again then any configuration applied via
+	 * {@link #enableNetwork(boolean)} etc must be applied again.
+	 * <p>
+	 * Do not call this method concurrently with {@link #start()}.
 	 */
-	void stop() throws IOException;
+	void stop() throws IOException, InterruptedException;
 
 	/**
 	 * Sets an observer for observing the state of the wrapper, replacing any
@@ -111,9 +125,26 @@ public interface TorWrapper {
 	enum TorState {
 
 		/**
-		 * The Tor process is either starting or stopping.
+		 * The wrapper has been created but the {@link #start()} method has not
+		 * yet been called. This is the initial state.
+		 */
+		NOT_STARTED,
+
+		/**
+		 * The {@link #start()} method has been called and the Tor process is
+		 * starting.
+		 */
+		STARTING,
+
+		/**
+		 * The {@link #start()} method has been called and the Tor process has
+		 * started.
+		 * <p>
+		 * No connections to the Tor network will be made in this state. The
+		 * wrapper remains in this state until {@link #enableNetwork(boolean)}
+		 * is called.
 		 */
-		STARTING_STOPPING,
+		STARTED,
 
 		/**
 		 * The Tor process has started, its network connection is enabled, and
@@ -131,7 +162,22 @@ public interface TorWrapper {
 		/**
 		 * The Tor process has started but its network connection is disabled.
 		 */
-		DISABLED
+		DISABLED,
+
+		/**
+		 * The {@link #stop()} method has been called and the Tor process is
+		 * stopping.
+		 */
+		STOPPING,
+
+		/**
+		 * The {@link #stop()} method has been called and the Tor process has
+		 * stopped.
+		 * <p>
+		 * A new Tor process can be started by calling the {@link #start()}
+		 * method again.
+		 */
+		STOPPED
 	}
 
 	/**

onionwrapper-java/src/main/java/org/briarproject/onionwrapper/JavaTorWrapper.java

@@ -20,14 +20,12 @@ abstract class JavaTorWrapper extends AbstractTorWrapper {
 			File torDirectory,
 			int torSocksPort,
 			int torControlPort) {
-		super(ioExecutor, eventExecutor, architecture, torDirectory,
-				torSocksPort, torControlPort);
+		super(ioExecutor, eventExecutor, architecture, torDirectory, torSocksPort, torControlPort);
 	}
 
 	@Override
 	protected long getLastUpdateTime() {
-		CodeSource codeSource =
-				getClass().getProtectionDomain().getCodeSource();
+		CodeSource codeSource = getClass().getProtectionDomain().getCodeSource();
 		if (codeSource == null) throw new AssertionError("CodeSource null");
 		try {
 			URI path = codeSource.getLocation().toURI();
@@ -39,8 +37,7 @@ abstract class JavaTorWrapper extends AbstractTorWrapper {
 	}
 
 	@Override
-	protected InputStream getResourceInputStream(String name,
-			String extension) {
+	protected InputStream getResourceInputStream(String name, String extension) {
 		ClassLoader cl = getClass().getClassLoader();
 		return requireNonNull(cl.getResourceAsStream(name + extension));
 	}

onionwrapper-java/src/main/java/org/briarproject/onionwrapper/WindowsTorWrapper.java

@@ -5,14 +5,8 @@ import com.sun.jna.platform.win32.Kernel32;
 import org.briarproject.nullsafety.NotNullByDefault;
 
 import java.io.File;
-import java.io.IOException;
-import java.util.Scanner;
-import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Executor;
 
-import static java.util.logging.Level.INFO;
-
 /**
  * A Tor wrapper for the Windows operating system.
  */
@@ -40,8 +34,7 @@ public class WindowsTorWrapper extends JavaTorWrapper {
 			File torDirectory,
 			int torSocksPort,
 			int torControlPort) {
-		super(ioExecutor, eventExecutor, architecture, torDirectory,
-				torSocksPort, torControlPort);
+		super(ioExecutor, eventExecutor, architecture, torDirectory, torSocksPort, torControlPort);
 	}
 
 	@Override
@@ -49,44 +42,6 @@ public class WindowsTorWrapper extends JavaTorWrapper {
 		return Kernel32.INSTANCE.GetCurrentProcessId();
 	}
 
-	@Override
-	protected void waitForTorToStart(Process torProcess)
-			throws InterruptedException, IOException {
-		// On Windows the RunAsDaemon option has no effect, so Tor won't detach.
-		// Wait for the control port to be opened, then continue to read its
-		// stdout and stderr in a background thread until it exits.
-		BlockingQueue<Boolean> success = new ArrayBlockingQueue<>(1);
-		ioExecutor.execute(() -> {
-			boolean started = false;
-			// Read the process's stdout (and redirected stderr)
-			Scanner stdout = new Scanner(torProcess.getInputStream());
-			// Log the first line of stdout (contains Tor and library versions)
-			if (stdout.hasNextLine()) LOG.info(stdout.nextLine());
-			// Startup has succeeded when the control port is open
-			while (stdout.hasNextLine()) {
-				String line = stdout.nextLine();
-				if (!started && line.contains("Opened Control listener")) {
-					success.add(true);
-					started = true;
-				}
-			}
-			stdout.close();
-			// If the control port wasn't opened, startup has failed
-			if (!started) success.add(false);
-			// Wait for the process to exit
-			try {
-				int exit = torProcess.waitFor();
-				if (LOG.isLoggable(INFO))
-					LOG.info("Tor exited with value " + exit);
-			} catch (InterruptedException e1) {
-				LOG.warning("Interrupted while waiting for Tor to exit");
-				Thread.currentThread().interrupt();
-			}
-		});
-		// Wait for the startup result
-		if (!success.take()) throw new IOException();
-	}
-
 	@Override
 	protected String getExecutableExtension() {
 		return ".exe";

onionwrapper-java/src/test/java/org/briarproject/onionwrapper/BootstrapTest.java

@@ -18,6 +18,9 @@ import static org.briarproject.onionwrapper.TestUtils.getTestDirectory;
 import static org.briarproject.onionwrapper.TestUtils.isLinux;
 import static org.briarproject.onionwrapper.TestUtils.isWindows;
 import static org.briarproject.onionwrapper.TorWrapper.TorState.CONNECTED;
+import static org.briarproject.onionwrapper.TorWrapper.TorState.STARTED;
+import static org.briarproject.onionwrapper.TorWrapper.TorState.STOPPED;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeNotNull;
 import static org.junit.Assume.assumeTrue;
@@ -28,7 +31,7 @@ public class BootstrapTest extends BaseTest {
 
 	private static final int SOCKS_PORT = 59060;
 	private static final int CONTROL_PORT = 59061;
-	private final static long TIMEOUT = MINUTES.toMillis(2);
+	private final static long TIMEOUT = MINUTES.toMillis(5);
 
 	private final ExecutorService executor = newCachedThreadPool();
 	private final File torDir = getTestDirectory();
@@ -59,9 +62,16 @@ public class BootstrapTest extends BaseTest {
 			throw new AssertionError("Running on unsupported OS");
 		}
 
+		// Bootstrap twice with the same wrapper to test wrapper reuse
+		testBootstrapping(tor);
+		testBootstrapping(tor);
+	}
+
+	private void testBootstrapping(TorWrapper tor) throws Exception {
 		boolean connected;
 		try {
 			tor.start();
+			assertEquals(STARTED, tor.getTorState());
 			tor.enableNetwork(true);
 			long start = System.currentTimeMillis();
 			while (System.currentTimeMillis() - start < TIMEOUT) {
@@ -74,6 +84,7 @@ public class BootstrapTest extends BaseTest {
 			else LOG.warning("Could not connect to Tor within timeout");
 		} finally {
 			tor.stop();
+			assertEquals(STOPPED, tor.getTorState());
 		}
 		assertTrue(connected);
 	}