Refactored PluginManager and Poller to remove non-open calls. Bug #15.

briar-api/src/org/briarproject/api/lifecycle/Service.java

@@ -2,9 +2,15 @@ package org.briarproject.api.lifecycle;
 
 public interface Service {
 
-	/** Starts the service and returns true if it started successfully. */
+	/**
+	 * Starts the service and returns true if it started successfully.
+	 * This method must not be called concurrently with {@link #stop()}.
+	 */
 	public boolean start();
 
-	/** Stops the service and returns true if it stopped successfully. */
+	/**
+	 * Stops the service and returns true if it stopped successfully.
+	 * This method must not be called concurrently with {@link #start()}.
+	 */
 	public boolean stop();
 }

briar-core/src/org/briarproject/plugins/PluginManagerImpl.java

@@ -42,8 +42,6 @@ import org.briarproject.api.plugins.simplex.SimplexPluginFactory;
 import org.briarproject.api.system.Clock;
 import org.briarproject.api.ui.UiCallback;
 
-// FIXME: Don't make alien calls with a lock held (that includes waiting on a
-// latch that depends on an alien call)
 class PluginManagerImpl implements PluginManager {
 
 	private static final Logger LOG =
@@ -63,7 +61,7 @@ class PluginManagerImpl implements PluginManager {
 
 	@Inject
 	PluginManagerImpl(@IoExecutor Executor ioExecutor,
-			SimplexPluginConfig simplexPluginConfig, 
+			SimplexPluginConfig simplexPluginConfig,
 			DuplexPluginConfig duplexPluginConfig, Clock clock,
 			DatabaseComponent db, Poller poller,
 			ConnectionManager connectionManager, UiCallback uiCallback) {
@@ -80,7 +78,7 @@ class PluginManagerImpl implements PluginManager {
 		duplexPlugins = new CopyOnWriteArrayList<DuplexPlugin>();
 	}
 
-	public synchronized boolean start() {
+	public boolean start() {
 		// Instantiate and start the simplex plugins
 		LOG.info("Starting simplex plugins");
 		Collection<SimplexPluginFactory> sFactories =
@@ -104,14 +102,10 @@ class PluginManagerImpl implements PluginManager {
 			Thread.currentThread().interrupt();
 			return false;
 		}
-		// Start the poller
-		LOG.info("Starting poller");
-		List<Plugin> start = new ArrayList<Plugin>(plugins.values());
-		poller.start(Collections.unmodifiableList(start));
 		return true;
 	}
 
-	public synchronized boolean stop() {
+	public boolean stop() {
 		// Stop the poller
 		LOG.info("Stopping poller");
 		poller.stop();
@@ -190,6 +184,7 @@ class PluginManagerImpl implements PluginManager {
 					if(started) {
 						plugins.put(id, plugin);
 						simplexPlugins.add(plugin);
+						if(plugin.shouldPoll()) poller.addPlugin(plugin);
 						if(LOG.isLoggable(INFO)) {
 							String name = plugin.getClass().getSimpleName();
 							LOG.info("Starting " + name + " took " +
@@ -252,6 +247,7 @@ class PluginManagerImpl implements PluginManager {
 					if(started) {
 						plugins.put(id, plugin);
 						duplexPlugins.add(plugin);
+						if(plugin.shouldPoll()) poller.addPlugin(plugin);
 						if(LOG.isLoggable(INFO)) {
 							String name = plugin.getClass().getSimpleName();
 							LOG.info("Starting " + name + " took " +

briar-core/src/org/briarproject/plugins/Poller.java

@@ -1,17 +1,15 @@
 package org.briarproject.plugins;
 
-import java.util.Collection;
-
 import org.briarproject.api.plugins.Plugin;
 
 interface Poller {
 
-	/** Starts a poller for the given collection of plugins. */
-	void start(Collection<Plugin> plugins);
-
-	/** Stops the poller. */
-	void stop();
+	/** Adds the given plugin to the collection of plugins to be polled. */
+	void addPlugin(Plugin p);
 
 	/** Tells the poller to poll the given plugin immediately. */
 	void pollNow(Plugin p);
+
+	/** Stops the poller. */
+	void stop();
 }

briar-core/src/org/briarproject/plugins/PollerImpl.java

@@ -2,7 +2,6 @@ package org.briarproject.plugins;
 
 import static java.util.logging.Level.INFO;
 
-import java.util.Collection;
 import java.util.TimerTask;
 import java.util.concurrent.Executor;
 import java.util.logging.Logger;
@@ -31,21 +30,19 @@ class PollerImpl implements Poller {
 		this.timer = timer;
 	}
 
-	public void start(Collection<Plugin> plugins) {
-		for(Plugin plugin : plugins) schedule(plugin, true);
+	public void stop() {
+		timer.cancel();
+	}
+
+	public void addPlugin(Plugin p) {
+		schedule(p, true);
 	}
 
 	private void schedule(Plugin plugin, boolean randomise) {
-		if(plugin.shouldPoll()) {
-			long interval = plugin.getPollingInterval();
-			// Randomise intervals at startup to spread out connection attempts
-			if(randomise) interval = (long) (interval * Math.random());
-			timer.schedule(new PollTask(plugin), interval);
-		}
-	}
-
-	public void stop() {
-		timer.cancel();
+		long interval = plugin.getPollingInterval();
+		// Randomise intervals at startup to spread out connection attempts
+		if(randomise) interval = (long) (interval * Math.random());
+		timer.schedule(new PollTask(plugin), interval);
 	}
 
 	public void pollNow(final Plugin p) {

briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java

@@ -76,6 +76,9 @@ public class PluginManagerImplTest extends BriarTestCase {
 			will(returnValue(true));
 			oneOf(simplexPlugin).start();
 			will(returnValue(true)); // Started
+			oneOf(simplexPlugin).shouldPoll();
+			will(returnValue(true));
+			oneOf(poller).addPlugin(simplexPlugin);
 			// Second simplex plugin
 			oneOf(simplexFailFactory).getId();
 			will(returnValue(simplexFailId));
@@ -102,14 +105,14 @@ public class PluginManagerImplTest extends BriarTestCase {
 			will(returnValue(true));
 			oneOf(duplexPlugin).start();
 			will(returnValue(true)); // Started
+			oneOf(duplexPlugin).shouldPoll();
+			will(returnValue(false));
 			// Second duplex plugin
 			oneOf(duplexFailFactory).getId();
 			will(returnValue(duplexFailId));
 			oneOf(duplexFailFactory).createPlugin(with(any(
 					DuplexPluginCallback.class)));
 			will(returnValue(null)); // Failed to create a plugin
-			// Start the poller
-			oneOf(poller).start(Arrays.asList(simplexPlugin, duplexPlugin));
 			// Stop the poller
 			oneOf(poller).stop();
 			// Stop the plugins