diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/lifecycle/LifecycleManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/lifecycle/LifecycleManager.java index 12bc36248..1f5a2c156 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/lifecycle/LifecycleManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/lifecycle/LifecycleManager.java @@ -37,8 +37,14 @@ public interface LifecycleManager { */ enum LifecycleState { - STARTING, MIGRATING_DATABASE, COMPACTING_DATABASE, STARTING_SERVICES, - RUNNING, STOPPING; + CREATED, + STARTING, + MIGRATING_DATABASE, + COMPACTING_DATABASE, + STARTING_SERVICES, + RUNNING, + STOPPING, + STOPPED; public boolean isAfter(LifecycleState state) { return ordinal() > state.ordinal(); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java index b55115fb7..bd2a35b99 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java @@ -18,7 +18,7 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Logger; import javax.annotation.concurrent.ThreadSafe; @@ -29,10 +29,12 @@ import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.COMPACTING_DATABASE; +import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.CREATED; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.MIGRATING_DATABASE; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.RUNNING; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.STARTING; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.STARTING_SERVICES; +import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.STOPPED; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.STOPPING; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.StartResult.ALREADY_RUNNING; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.StartResult.CLOCK_ERROR; @@ -60,12 +62,11 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { private final List services; private final List openDatabaseHooks; private final List executors; - private final Semaphore startStopSemaphore = new Semaphore(1); private final CountDownLatch dbLatch = new CountDownLatch(1); private final CountDownLatch startupLatch = new CountDownLatch(1); private final CountDownLatch shutdownLatch = new CountDownLatch(1); - - private volatile LifecycleState state = STARTING; + private final AtomicReference state = + new AtomicReference<>(CREATED); @Inject LifecycleManagerImpl(DatabaseComponent db, EventBus eventBus, @@ -102,8 +103,8 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { @Override public StartResult startServices(SecretKey dbKey) { - if (!startStopSemaphore.tryAcquire()) { - LOG.info("Already starting or stopping"); + if (!state.compareAndSet(CREATED, STARTING)) { + LOG.warning("Already running"); return ALREADY_RUNNING; } long now = clock.currentTimeMillis(); @@ -135,7 +136,7 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { }); LOG.info("Starting services"); - state = STARTING_SERVICES; + state.set(STARTING_SERVICES); dbLatch.countDown(); eventBus.broadcast(new LifecycleEvent(STARTING_SERVICES)); @@ -148,7 +149,7 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { } } - state = RUNNING; + state.set(RUNNING); startupLatch.countDown(); eventBus.broadcast(new LifecycleEvent(RUNNING)); return SUCCESS; @@ -164,63 +165,58 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { } catch (ServiceException e) { logException(LOG, WARNING, e); return SERVICE_ERROR; - } finally { - startStopSemaphore.release(); } } @Override public void onDatabaseMigration() { - state = MIGRATING_DATABASE; + state.set(MIGRATING_DATABASE); eventBus.broadcast(new LifecycleEvent(MIGRATING_DATABASE)); } @Override public void onDatabaseCompaction() { - state = COMPACTING_DATABASE; + state.set(COMPACTING_DATABASE); eventBus.broadcast(new LifecycleEvent(COMPACTING_DATABASE)); } @Override public void stopServices() { - try { - startStopSemaphore.acquire(); - } catch (InterruptedException e) { - LOG.warning("Interrupted while waiting to stop services"); + if (!state.compareAndSet(RUNNING, STOPPING)) { + LOG.warning("Not running"); return; } - try { - if (state == STOPPING) { - LOG.info("Already stopped"); - return; - } - LOG.info("Stopping services"); - state = STOPPING; - eventBus.broadcast(new LifecycleEvent(STOPPING)); - for (Service s : services) { + LOG.info("Stopping services"); + eventBus.broadcast(new LifecycleEvent(STOPPING)); + for (Service s : services) { + try { long start = now(); s.stopService(); if (LOG.isLoggable(FINE)) { logDuration(LOG, "Stopping service " + s.getClass().getSimpleName(), start); } + } catch (ServiceException e) { + logException(LOG, WARNING, e); } - for (ExecutorService e : executors) { - if (LOG.isLoggable(FINE)) { - LOG.fine("Stopping executor " - + e.getClass().getSimpleName()); - } - e.shutdownNow(); + } + for (ExecutorService e : executors) { + if (LOG.isLoggable(FINE)) { + LOG.fine("Stopping executor " + + e.getClass().getSimpleName()); } + e.shutdownNow(); + } + try { long start = now(); db.close(); logDuration(LOG, "Closing database", start); - shutdownLatch.countDown(); - } catch (DbException | ServiceException e) { + } catch (DbException e) { logException(LOG, WARNING, e); - } finally { - startStopSemaphore.release(); } + state.set(STOPPED); + shutdownLatch.countDown(); + eventBus.broadcast(new LifecycleEvent(STOPPED)); } @Override @@ -240,6 +236,6 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { @Override public LifecycleState getLifecycleState() { - return state; + return state.get(); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java index 0b4bfbcef..2721cd5a2 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/lifecycle/LifecycleManagerImplTest.java @@ -5,6 +5,7 @@ import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.lifecycle.LifecycleManager.OpenDatabaseHook; +import org.briarproject.bramble.api.lifecycle.Service; import org.briarproject.bramble.api.lifecycle.event.LifecycleEvent; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.test.BrambleMockTestCase; @@ -12,12 +13,10 @@ import org.briarproject.bramble.test.DbExpectations; import org.jmock.Expectations; import org.junit.Test; -import java.util.concurrent.atomic.AtomicBoolean; - -import static junit.framework.TestCase.assertTrue; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.RUNNING; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.STARTING; -import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.STOPPING; +import static org.briarproject.bramble.api.lifecycle.LifecycleManager.LifecycleState.STOPPED; +import static org.briarproject.bramble.api.lifecycle.LifecycleManager.StartResult.ALREADY_RUNNING; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.StartResult.CLOCK_ERROR; import static org.briarproject.bramble.api.lifecycle.LifecycleManager.StartResult.SUCCESS; import static org.briarproject.bramble.api.system.Clock.MAX_REASONABLE_TIME_MS; @@ -30,6 +29,8 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { private final DatabaseComponent db = context.mock(DatabaseComponent.class); private final EventBus eventBus = context.mock(EventBus.class); private final Clock clock = context.mock(Clock.class); + private final OpenDatabaseHook hook = context.mock(OpenDatabaseHook.class); + private final Service service = context.mock(Service.class); private final SecretKey dbKey = getSecretKey(); @@ -40,8 +41,6 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { public void testOpenDatabaseHooksAreCalledAtStartup() throws Exception { long now = System.currentTimeMillis(); Transaction txn = new Transaction(null, false); - AtomicBoolean called = new AtomicBoolean(false); - OpenDatabaseHook hook = transaction -> called.set(true); context.checking(new DbExpectations() {{ oneOf(clock).currentTimeMillis(); @@ -50,6 +49,7 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { will(returnValue(false)); oneOf(db).transaction(with(false), withDbRunnable(txn)); oneOf(db).removeTemporaryMessages(txn); + oneOf(hook).onDatabaseOpened(txn); allowing(eventBus).broadcast(with(any(LifecycleEvent.class))); }}); @@ -57,7 +57,38 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { assertEquals(SUCCESS, lifecycleManager.startServices(dbKey)); assertEquals(RUNNING, lifecycleManager.getLifecycleState()); - assertTrue(called.get()); + } + + @Test + public void testServicesAreStartedAndStopped() throws Exception { + long now = System.currentTimeMillis(); + Transaction txn = new Transaction(null, false); + + context.checking(new DbExpectations() {{ + oneOf(clock).currentTimeMillis(); + will(returnValue(now)); + oneOf(db).open(dbKey, lifecycleManager); + will(returnValue(false)); + oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(db).removeTemporaryMessages(txn); + oneOf(service).startService(); + allowing(eventBus).broadcast(with(any(LifecycleEvent.class))); + }}); + + lifecycleManager.registerService(service); + + assertEquals(SUCCESS, lifecycleManager.startServices(dbKey)); + assertEquals(RUNNING, lifecycleManager.getLifecycleState()); + context.assertIsSatisfied(); + + context.checking(new Expectations() {{ + oneOf(db).close(); + oneOf(service).stopService(); + allowing(eventBus).broadcast(with(any(LifecycleEvent.class))); + }}); + + lifecycleManager.stopServices(); + assertEquals(STOPPED, lifecycleManager.getLifecycleState()); } @Test @@ -84,6 +115,31 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { assertEquals(STARTING, lifecycleManager.getLifecycleState()); } + @Test + public void testSecondCallToStartServicesReturnsEarly() throws Exception { + long now = System.currentTimeMillis(); + Transaction txn = new Transaction(null, false); + + context.checking(new DbExpectations() {{ + oneOf(clock).currentTimeMillis(); + will(returnValue(now)); + oneOf(db).open(dbKey, lifecycleManager); + will(returnValue(false)); + oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(db).removeTemporaryMessages(txn); + allowing(eventBus).broadcast(with(any(LifecycleEvent.class))); + }}); + + assertEquals(SUCCESS, lifecycleManager.startServices(dbKey)); + assertEquals(RUNNING, lifecycleManager.getLifecycleState()); + context.assertIsSatisfied(); + + // Calling startServices() again should not try to open the DB or + // start the services again + assertEquals(ALREADY_RUNNING, lifecycleManager.startServices(dbKey)); + assertEquals(RUNNING, lifecycleManager.getLifecycleState()); + } + @Test public void testSecondCallToStopServicesReturnsEarly() throws Exception { long now = System.currentTimeMillis(); @@ -96,7 +152,7 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { will(returnValue(false)); oneOf(db).transaction(with(false), withDbRunnable(txn)); oneOf(db).removeTemporaryMessages(txn); - exactly(2).of(eventBus).broadcast(with(any(LifecycleEvent.class))); + allowing(eventBus).broadcast(with(any(LifecycleEvent.class))); }}); assertEquals(SUCCESS, lifecycleManager.startServices(dbKey)); @@ -104,17 +160,17 @@ public class LifecycleManagerImplTest extends BrambleMockTestCase { context.assertIsSatisfied(); context.checking(new Expectations() {{ - oneOf(eventBus).broadcast(with(any(LifecycleEvent.class))); oneOf(db).close(); + allowing(eventBus).broadcast(with(any(LifecycleEvent.class))); }}); lifecycleManager.stopServices(); - assertEquals(STOPPING, lifecycleManager.getLifecycleState()); + assertEquals(STOPPED, lifecycleManager.getLifecycleState()); context.assertIsSatisfied(); // Calling stopServices() again should not broadcast another event or // try to close the DB again lifecycleManager.stopServices(); - assertEquals(STOPPING, lifecycleManager.getLifecycleState()); + assertEquals(STOPPED, lifecycleManager.getLifecycleState()); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/BriarService.java b/briar-android/src/main/java/org/briarproject/briar/android/BriarService.java index 70be9f0e9..58e9adf68 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/BriarService.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/BriarService.java @@ -156,8 +156,11 @@ public class BriarService extends Service { if (result == SUCCESS) { started = true; } else if (result == ALREADY_RUNNING) { - LOG.info("Already running"); - stopSelf(); + LOG.warning("Already running"); + // The core has outlived the original BriarService + // instance. We don't know how to recover from this + // unexpected state, so try to exit cleanly + shutdownFromBackground(); } else { if (LOG.isLoggable(WARNING)) LOG.warning("Startup failed: " + result);