Merge branch '273-service-exceptions' into 'master'

Services should throw exceptions for startup errors

Fixes #273

See merge request !134

briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java

@@ -10,10 +10,10 @@ import android.support.v4.app.NotificationCompat;
 import android.support.v4.app.TaskStackBuilder;
 
 import org.briarproject.R;
-import org.briarproject.android.contact.ConversationActivity;
-import org.briarproject.android.forum.ForumActivity;
 import org.briarproject.android.api.AndroidExecutor;
 import org.briarproject.android.api.AndroidNotificationManager;
+import org.briarproject.android.contact.ConversationActivity;
+import org.briarproject.android.forum.ForumActivity;
 import org.briarproject.api.db.DatabaseExecutor;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.event.Event;
@@ -22,6 +22,7 @@ import org.briarproject.api.event.MessageValidatedEvent;
 import org.briarproject.api.event.SettingsUpdatedEvent;
 import org.briarproject.api.forum.ForumManager;
 import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
 import org.briarproject.api.messaging.MessagingManager;
 import org.briarproject.api.settings.Settings;
 import org.briarproject.api.settings.SettingsManager;
@@ -31,7 +32,10 @@ import org.briarproject.util.StringUtils;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
+import java.util.concurrent.Future;
 import java.util.logging.Logger;
 
 import javax.inject.Inject;
@@ -93,37 +97,30 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
 	}
 
 	@Override
-	public boolean start() {
+	public void startService() throws ServiceException {
-		loadSettings();
+		try {
-		return true;
+			settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
-	}
+		} catch (DbException e) {
-
+			throw new ServiceException(e);
-	private void loadSettings() {
+		}
-		dbExecutor.execute(new Runnable() {
-			public void run() {
-				try {
-					settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
-				} catch (DbException e) {
-					if (LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
-				}
-			}
-		});
 	}
 
 	@Override
-	public boolean stop() {
+	public void stopService() throws ServiceException {
-		clearNotifications();
+		Future<Void> f = androidExecutor.submit(new Callable<Void>() {
-		return true;
+			public Void call() {
-	}
-
-	private void clearNotifications() {
-		androidExecutor.execute(new Runnable() {
-			public void run() {
 				clearPrivateMessageNotification();
 				clearForumPostNotification();
+				return null;
 			}
 		});
+		try {
+			f.get();
+		} catch (InterruptedException e) {
+			throw new ServiceException(e);
+		} catch (ExecutionException e) {
+			throw new ServiceException(e);
+		}
 	}
 
 	private void clearPrivateMessageNotification() {
@@ -154,6 +151,19 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
 		}
 	}
 
+	private void loadSettings() {
+		dbExecutor.execute(new Runnable() {
+			public void run() {
+				try {
+					settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
+				} catch (DbException e) {
+					if (LOG.isLoggable(WARNING))
+						LOG.log(WARNING, e.toString(), e);
+				}
+			}
+		});
+	}
+
 	public void showPrivateMessageNotification(final GroupId g) {
 		androidExecutor.execute(new Runnable() {
 			public void run() {

briar-api/src/org/briarproject/api/db/DatabaseComponent.java

@@ -19,7 +19,6 @@ import org.briarproject.api.sync.Offer;
 import org.briarproject.api.sync.Request;
 import org.briarproject.api.transport.TransportKeys;
 
-import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
 
@@ -37,7 +36,7 @@ public interface DatabaseComponent {
 	/**
 	 * Waits for any open transactions to finish and closes the database.
 	 */
-	void close() throws DbException, IOException;
+	void close() throws DbException;
 
 	/**
 	 * Starts a new transaction and returns an object representing it.

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

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

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

@@ -0,0 +1,15 @@
+package org.briarproject.api.lifecycle;
+
+/**
+ * An exception that indicates an error starting or stopping a {@link Service}.
+ */
+public class ServiceException extends Exception {
+
+	public ServiceException() {
+		super();
+	}
+
+	public ServiceException(Throwable cause) {
+		super(cause);
+	}
+}

briar-core/src/org/briarproject/db/Database.java

@@ -19,7 +19,6 @@ import org.briarproject.api.sync.MessageStatus;
 import org.briarproject.api.sync.ValidationManager.Validity;
 import org.briarproject.api.transport.TransportKeys;
 
-import java.io.IOException;
 import java.util.Collection;
 import java.util.Map;
 
@@ -41,7 +40,7 @@ interface Database<T> {
 	 * Prevents new transactions from starting, waits for all current
 	 * transactions to finish, and closes the database.
 	 */
-	void close() throws DbException, IOException;
+	void close() throws DbException;
 
 	/**
 	 * Starts a new transaction and returns an object representing it.

briar-core/src/org/briarproject/db/DatabaseComponentImpl.java

@@ -50,7 +50,6 @@ import org.briarproject.api.sync.Request;
 import org.briarproject.api.sync.ValidationManager.Validity;
 import org.briarproject.api.transport.TransportKeys;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -101,9 +100,6 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 				} catch (DbException e) {
 					if (LOG.isLoggable(WARNING))
 						LOG.log(WARNING, e.toString(), e);
-				} catch (IOException e) {
-					if (LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
 				}
 			}
 		};
@@ -112,7 +108,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		return reopened;
 	}
 
-	public void close() throws DbException, IOException {
+	public void close() throws DbException {
 		if (closed.getAndSet(true)) return;
 		shutdown.removeShutdownHook(shutdownHandle);
 		db.close();

briar-core/src/org/briarproject/lifecycle/LifecycleManagerImpl.java

@@ -8,8 +8,8 @@ import org.briarproject.api.event.EventBus;
 import org.briarproject.api.event.ShutdownEvent;
 import org.briarproject.api.lifecycle.LifecycleManager;
 import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
 
-import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
@@ -101,16 +101,11 @@ class LifecycleManagerImpl implements LifecycleManager {
 			}
 			for (Service s : services) {
 				start = System.currentTimeMillis();
-				boolean started = s.start();
+				s.startService();
 				duration = System.currentTimeMillis() - start;
-				if (!started) {
-					if (LOG.isLoggable(WARNING))
-						LOG.warning(s.getClass().getName() + " did not start");
-					return SERVICE_ERROR;
-				}
 				if (LOG.isLoggable(INFO)) {
-					String name = s.getClass().getName();
+					LOG.info("Starting " + s.getClass().getName()
-					LOG.info("Starting " + name + " took " + duration + " ms");
+							+ " took " + duration + " ms");
 				}
 			}
 			startupLatch.countDown();
@@ -118,6 +113,9 @@ class LifecycleManagerImpl implements LifecycleManager {
 		} catch (DbException e) {
 			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			return DB_ERROR;
+		} catch (ServiceException e) {
+			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			return SERVICE_ERROR;
 		} finally {
 			startStopSemaphore.release();
 		}
@@ -134,12 +132,9 @@ class LifecycleManagerImpl implements LifecycleManager {
 			LOG.info("Stopping services");
 			eventBus.broadcast(new ShutdownEvent());
 			for (Service s : services) {
-				boolean stopped = s.stop();
+				s.stopService();
-				if (LOG.isLoggable(INFO)) {
+				if (LOG.isLoggable(INFO))
-					String name = s.getClass().getName();
+					LOG.info("Service stopped: " + s.getClass().getName());
-					if (stopped) LOG.info("Service stopped: " + name);
-					else LOG.warning("Service failed to stop: " + name);
-				}
 			}
 			for (ExecutorService e : executors) e.shutdownNow();
 			if (LOG.isLoggable(INFO))
@@ -149,7 +144,7 @@ class LifecycleManagerImpl implements LifecycleManager {
 			shutdownLatch.countDown();
 		} catch (DbException e) {
 			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
-		} catch (IOException e) {
+		} catch (ServiceException e) {
 			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 		} finally {
 			startStopSemaphore.release();

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

@@ -8,6 +8,7 @@ import org.briarproject.api.event.TransportDisabledEvent;
 import org.briarproject.api.event.TransportEnabledEvent;
 import org.briarproject.api.lifecycle.IoExecutor;
 import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
 import org.briarproject.api.plugins.ConnectionManager;
 import org.briarproject.api.plugins.Plugin;
 import org.briarproject.api.plugins.PluginCallback;
@@ -83,7 +84,7 @@ class PluginManagerImpl implements PluginManager, Service {
 	}
 
 	@Override
-	public boolean start() {
+	public void startService() throws ServiceException {
 		// Instantiate and start the simplex plugins
 		LOG.info("Starting simplex plugins");
 		Collection<SimplexPluginFactory> sFactories =
@@ -103,15 +104,12 @@ class PluginManagerImpl implements PluginManager, Service {
 			sLatch.await();
 			dLatch.await();
 		} catch (InterruptedException e) {
-			LOG.warning("Interrupted while starting plugins");
+			throw new ServiceException(e);
-			Thread.currentThread().interrupt();
-			return false;
 		}
-		return true;
 	}
 
 	@Override
-	public boolean stop() {
+	public void stopService() throws ServiceException {
 		// Stop the poller
 		LOG.info("Stopping poller");
 		poller.stop();
@@ -131,11 +129,8 @@ class PluginManagerImpl implements PluginManager, Service {
 		try {
 			latch.await();
 		} catch (InterruptedException e) {
-			LOG.warning("Interrupted while stopping plugins");
+			throw new ServiceException(e);
-			Thread.currentThread().interrupt();
-			return false;
 		}
-		return true;
 	}
 
 	public Plugin getPlugin(TransportId t) {

briar-core/src/org/briarproject/sync/ValidationManagerImpl.java

@@ -57,14 +57,12 @@ class ValidationManagerImpl implements ValidationManager, Service,
 	}
 
 	@Override
-	public boolean start() {
+	public void startService() {
 		for (ClientId c : validators.keySet()) getMessagesToValidate(c);
-		return true;
 	}
 
 	@Override
-	public boolean stop() {
+	public void stopService() {
-		return true;
 	}
 
 	@Override

briar-core/src/org/briarproject/transport/KeyManagerImpl.java

@@ -14,6 +14,7 @@ import org.briarproject.api.event.ContactStatusChangedEvent;
 import org.briarproject.api.event.Event;
 import org.briarproject.api.event.EventListener;
 import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
 import org.briarproject.api.plugins.PluginConfig;
 import org.briarproject.api.plugins.duplex.DuplexPluginFactory;
 import org.briarproject.api.plugins.simplex.SimplexPluginFactory;
@@ -32,7 +33,6 @@ import java.util.logging.Logger;
 import javax.inject.Inject;
 
 import static java.util.logging.Level.INFO;
-import static java.util.logging.Level.WARNING;
 
 class KeyManagerImpl implements KeyManager, Service, EventListener {
 
@@ -64,7 +64,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
 	}
 
 	@Override
-	public boolean start() {
+	public void startService() throws ServiceException {
 		Map<TransportId, Integer> transports =
 				new HashMap<TransportId, Integer>();
 		for (SimplexPluginFactory f : pluginConfig.getSimplexFactories())
@@ -89,15 +89,12 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
 				db.endTransaction(txn);
 			}
 		} catch (DbException e) {
-			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			throw new ServiceException(e);
-			return false;
 		}
-		return true;
 	}
 
 	@Override
-	public boolean stop() {
+	public void stopService() {
-		return true;
 	}
 
 	public void addContact(Transaction txn, ContactId c, SecretKey master,

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

@@ -23,8 +23,6 @@ import java.util.Arrays;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 
-import static org.junit.Assert.assertTrue;
-
 public class PluginManagerImplTest extends BriarTestCase {
 
 	@Test
@@ -117,8 +115,8 @@ public class PluginManagerImplTest extends BriarTestCase {
 				transportPropertyManager, uiCallback);
 
 		// Two plugins should be started and stopped
-		assertTrue(p.start());
+		p.startService();
-		assertTrue(p.stop());
+		p.stopService();
 
 		context.assertIsSatisfied();
 	}

briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java

@@ -112,7 +112,7 @@ public class ValidationManagerImplTest extends BriarTestCase {
 				cryptoExecutor);
 		vm.registerMessageValidator(clientId, validator);
 		vm.registerIncomingMessageHook(clientId, hook);
-		vm.start();
+		vm.startService();
 
 		context.assertIsSatisfied();
 	}
@@ -166,7 +166,7 @@ public class ValidationManagerImplTest extends BriarTestCase {
 				cryptoExecutor);
 		vm.registerMessageValidator(clientId, validator);
 		vm.registerIncomingMessageHook(clientId, hook);
-		vm.start();
+		vm.startService();
 
 		context.assertIsSatisfied();
 	}
@@ -223,7 +223,7 @@ public class ValidationManagerImplTest extends BriarTestCase {
 				cryptoExecutor);
 		vm.registerMessageValidator(clientId, validator);
 		vm.registerIncomingMessageHook(clientId, hook);
-		vm.start();
+		vm.startService();
 
 		context.assertIsSatisfied();
 	}