From ff8301521cb073d83bbcb85b5d8459b19829af79 Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Thu, 5 May 2016 11:07:58 +0100 Subject: [PATCH] Replaced Timer with ScheduledExecutorService. #258 --- .../IntroductionIntegrationTest.java | 5 +- .../IntroductionIntegrationTestComponent.java | 7 +- .../MessageSizeIntegrationTest.java | 2 + .../MessageSizeIntegrationTestComponent.java | 9 ++- .../SimplexMessagingIntegrationTest.java | 4 + ...plexMessagingIntegrationTestComponent.java | 6 +- .../SyncIntegrationTestComponent.java | 2 +- .../system/AndroidSystemModule.java | 1 + .../org/briarproject/api/system/Timer.java | 27 ------- .../org/briarproject/CoreEagerSingletons.java | 3 + .../src/org/briarproject/CoreModule.java | 1 + .../plugins/PluginManagerImpl.java | 5 +- .../briarproject/plugins/PluginsModule.java | 9 +-- .../src/org/briarproject/plugins/Poller.java | 3 - .../org/briarproject/plugins/PollerImpl.java | 30 +++---- .../org/briarproject/system/SystemModule.java | 30 +++++-- .../org/briarproject/system/SystemTimer.java | 31 ------- .../transport/KeyManagerImpl.java | 19 +++-- .../transport/TransportKeyManager.java | 33 +++++--- .../transport/TransportModule.java | 5 +- .../system/DesktopSeedProviderModule.java | 19 +++++ .../system/DesktopSystemModule.java | 31 ------- .../briarproject/TestSeedProviderModule.java | 18 +++++ .../org/briarproject/TestSystemModule.java | 29 ------- .../plugins/PluginManagerImplTest.java | 4 - .../transport/TransportKeyManagerTest.java | 80 ++++++++++++------- 26 files changed, 201 insertions(+), 212 deletions(-) delete mode 100644 briar-api/src/org/briarproject/api/system/Timer.java delete mode 100644 briar-core/src/org/briarproject/system/SystemTimer.java create mode 100644 briar-desktop/src/org/briarproject/system/DesktopSeedProviderModule.java delete mode 100644 briar-desktop/src/org/briarproject/system/DesktopSystemModule.java create mode 100644 briar-tests/src/org/briarproject/TestSeedProviderModule.java delete mode 100644 briar-tests/src/org/briarproject/TestSystemModule.java diff --git a/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java index cb219c3229..f049c142a8 100644 --- a/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTest.java @@ -42,6 +42,7 @@ import org.briarproject.introduction.MessageSender; import org.briarproject.lifecycle.LifecycleModule; import org.briarproject.properties.PropertiesModule; import org.briarproject.sync.SyncModule; +import org.briarproject.system.SystemModule; import org.briarproject.transport.TransportModule; import org.junit.After; import org.junit.Before; @@ -944,6 +945,7 @@ public class IntroductionIntegrationTest extends BriarTestCase { this.accept = accept; } + @Override public void eventOccurred(Event e) { if (e instanceof MessageValidatedEvent) { MessageValidatedEvent event = (MessageValidatedEvent) e; @@ -1010,6 +1012,7 @@ public class IntroductionIntegrationTest extends BriarTestCase { public volatile boolean response2Received = false; public volatile boolean aborted = false; + @Override public void eventOccurred(Event e) { if (e instanceof MessageValidatedEvent) { MessageValidatedEvent event = (MessageValidatedEvent) e; @@ -1050,7 +1053,7 @@ public class IntroductionIntegrationTest extends BriarTestCase { component.inject(new ContactModule.EagerSingletons()); component.inject(new TransportModule.EagerSingletons()); component.inject(new SyncModule.EagerSingletons()); + component.inject(new SystemModule.EagerSingletons()); component.inject(new PropertiesModule.EagerSingletons()); } - } diff --git a/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTestComponent.java b/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTestComponent.java index 1a08532914..733082ef0b 100644 --- a/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTestComponent.java +++ b/briar-android-tests/src/test/java/org/briarproject/IntroductionIntegrationTestComponent.java @@ -21,6 +21,7 @@ import org.briarproject.introduction.MessageSender; import org.briarproject.lifecycle.LifecycleModule; import org.briarproject.properties.PropertiesModule; import org.briarproject.sync.SyncModule; +import org.briarproject.system.SystemModule; import org.briarproject.transport.TransportModule; import javax.inject.Singleton; @@ -29,9 +30,9 @@ import dagger.Component; @Singleton @Component(modules = { - TestSystemModule.class, TestDatabaseModule.class, TestPluginsModule.class, + TestSeedProviderModule.class, LifecycleModule.class, IntroductionModule.class, DatabaseModule.class, @@ -42,6 +43,7 @@ import dagger.Component; TransportModule.class, ClientsModule.class, SyncModule.class, + SystemModule.class, DataModule.class, PropertiesModule.class }) @@ -61,6 +63,8 @@ public interface IntroductionIntegrationTestComponent { void inject(SyncModule.EagerSingletons init); + void inject(SystemModule.EagerSingletons init); + void inject(TransportModule.EagerSingletons init); LifecycleManager getLifecycleManager(); @@ -84,5 +88,4 @@ public interface IntroductionIntegrationTestComponent { MessageSender getMessageSender(); IntroductionGroupFactory getIntroductionGroupFactory(); - } diff --git a/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java index 01157a3e80..5e5b877cc8 100644 --- a/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTest.java @@ -13,6 +13,7 @@ import org.briarproject.api.messaging.PrivateMessage; import org.briarproject.api.messaging.PrivateMessageFactory; import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.MessageId; +import org.briarproject.system.SystemModule; import org.junit.Test; import javax.inject.Inject; @@ -39,6 +40,7 @@ public class MessageSizeIntegrationTest extends BriarTestCase { MessageSizeIntegrationTestComponent component = DaggerMessageSizeIntegrationTestComponent.builder().build(); component.inject(this); + component.inject(new SystemModule.EagerSingletons()); } @Test diff --git a/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTestComponent.java b/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTestComponent.java index 9ec86f7c7f..fe53046406 100644 --- a/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTestComponent.java +++ b/briar-android-tests/src/test/java/org/briarproject/MessageSizeIntegrationTestComponent.java @@ -9,6 +9,7 @@ import org.briarproject.forum.ForumModule; import org.briarproject.identity.IdentityModule; import org.briarproject.messaging.MessagingModule; import org.briarproject.sync.SyncModule; +import org.briarproject.system.SystemModule; import javax.inject.Singleton; @@ -18,7 +19,7 @@ import dagger.Component; @Component(modules = { TestDatabaseModule.class, TestLifecycleModule.class, - TestSystemModule.class, + TestSeedProviderModule.class, ClientsModule.class, CryptoModule.class, DataModule.class, @@ -27,8 +28,12 @@ import dagger.Component; ForumModule.class, IdentityModule.class, MessagingModule.class, - SyncModule.class + SyncModule.class, + SystemModule.class }) public interface MessageSizeIntegrationTestComponent { + void inject(MessageSizeIntegrationTest testCase); + + void inject(SystemModule.EagerSingletons init); } diff --git a/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTest.java b/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTest.java index 8a2d43e2c4..3cbf69a6da 100644 --- a/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTest.java +++ b/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTest.java @@ -21,6 +21,7 @@ import org.briarproject.api.transport.KeyManager; import org.briarproject.api.transport.StreamContext; import org.briarproject.api.transport.StreamReaderFactory; import org.briarproject.api.transport.StreamWriterFactory; +import org.briarproject.system.SystemModule; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -57,8 +58,10 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase { assertTrue(testDir.mkdirs()); alice = DaggerSimplexMessagingIntegrationTestComponent.builder() .testDatabaseModule(new TestDatabaseModule(aliceDir)).build(); + alice.inject(new SystemModule.EagerSingletons()); bob = DaggerSimplexMessagingIntegrationTestComponent.builder() .testDatabaseModule(new TestDatabaseModule(bobDir)).build(); + bob.inject(new SystemModule.EagerSingletons()); } @Test @@ -183,6 +186,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase { private volatile boolean messageAdded = false; + @Override public void eventOccurred(Event e) { if (e instanceof MessageAddedEvent) messageAdded = true; } diff --git a/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTestComponent.java b/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTestComponent.java index 28480fb1a1..9de8e9c5ea 100644 --- a/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTestComponent.java +++ b/briar-android-tests/src/test/java/org/briarproject/SimplexMessagingIntegrationTestComponent.java @@ -21,6 +21,7 @@ import org.briarproject.lifecycle.LifecycleModule; import org.briarproject.messaging.MessagingModule; import org.briarproject.plugins.PluginsModule; import org.briarproject.sync.SyncModule; +import org.briarproject.system.SystemModule; import org.briarproject.transport.TransportModule; import javax.inject.Singleton; @@ -31,7 +32,7 @@ import dagger.Component; @Component(modules = { TestDatabaseModule.class, TestPluginsModule.class, - TestSystemModule.class, + TestSeedProviderModule.class, ClientsModule.class, ContactModule.class, CryptoModule.class, @@ -43,11 +44,12 @@ import dagger.Component; MessagingModule.class, PluginsModule.class, SyncModule.class, + SystemModule.class, TransportModule.class }) public interface SimplexMessagingIntegrationTestComponent { - void inject(SimplexMessagingIntegrationTest testCase); + void inject(SystemModule.EagerSingletons init); LifecycleManager getLifecycleManager(); diff --git a/briar-android-tests/src/test/java/org/briarproject/SyncIntegrationTestComponent.java b/briar-android-tests/src/test/java/org/briarproject/SyncIntegrationTestComponent.java index b5d55b9357..69710bd5a6 100644 --- a/briar-android-tests/src/test/java/org/briarproject/SyncIntegrationTestComponent.java +++ b/briar-android-tests/src/test/java/org/briarproject/SyncIntegrationTestComponent.java @@ -10,7 +10,7 @@ import dagger.Component; @Singleton @Component(modules = { - TestSystemModule.class, + TestSeedProviderModule.class, CryptoModule.class, SyncModule.class, TransportModule.class diff --git a/briar-android/src/org/briarproject/system/AndroidSystemModule.java b/briar-android/src/org/briarproject/system/AndroidSystemModule.java index f6fb19f16d..a577ac7c3e 100644 --- a/briar-android/src/org/briarproject/system/AndroidSystemModule.java +++ b/briar-android/src/org/briarproject/system/AndroidSystemModule.java @@ -15,6 +15,7 @@ import dagger.Provides; public class AndroidSystemModule { @Provides + @Singleton public SeedProvider provideSeedProvider(Application app) { return new AndroidSeedProvider(app); } diff --git a/briar-api/src/org/briarproject/api/system/Timer.java b/briar-api/src/org/briarproject/api/system/Timer.java deleted file mode 100644 index 9eb08e88b1..0000000000 --- a/briar-api/src/org/briarproject/api/system/Timer.java +++ /dev/null @@ -1,27 +0,0 @@ -package org.briarproject.api.system; - -import java.util.TimerTask; - -/** - * A wrapper around a {@link java.util.Timer} that allows it to be replaced for - * testing. - */ -public interface Timer { - - /** @see {@link java.util.Timer#cancel()} */ - void cancel(); - - /** @see {@link java.util.Timer#purge()} */ - int purge(); - - /** @see {@link java.util.Timer#schedule(TimerTask, long)} */ - void schedule(TimerTask task, long delay); - - /** @see {@link java.util.Timer#schedule(TimerTask, long, long)} */ - void schedule(TimerTask task, long delay, long period); - - /** - * @see {@link java.util.Timer#scheduleAtFixedRate(TimerTask, long, long)} - */ - void scheduleAtFixedRate(TimerTask task, long delay, long period); -} diff --git a/briar-core/src/org/briarproject/CoreEagerSingletons.java b/briar-core/src/org/briarproject/CoreEagerSingletons.java index 0823bccbf2..174ee9c570 100644 --- a/briar-core/src/org/briarproject/CoreEagerSingletons.java +++ b/briar-core/src/org/briarproject/CoreEagerSingletons.java @@ -10,6 +10,7 @@ import org.briarproject.messaging.MessagingModule; import org.briarproject.plugins.PluginsModule; import org.briarproject.properties.PropertiesModule; import org.briarproject.sync.SyncModule; +import org.briarproject.system.SystemModule; import org.briarproject.transport.TransportModule; public interface CoreEagerSingletons { @@ -34,5 +35,7 @@ public interface CoreEagerSingletons { void inject(SyncModule.EagerSingletons init); + void inject(SystemModule.EagerSingletons init); + void inject(TransportModule.EagerSingletons init); } diff --git a/briar-core/src/org/briarproject/CoreModule.java b/briar-core/src/org/briarproject/CoreModule.java index 62828dfffb..238c859e7f 100644 --- a/briar-core/src/org/briarproject/CoreModule.java +++ b/briar-core/src/org/briarproject/CoreModule.java @@ -61,6 +61,7 @@ public class CoreModule { c.inject(new PluginsModule.EagerSingletons()); c.inject(new PropertiesModule.EagerSingletons()); c.inject(new SyncModule.EagerSingletons()); + c.inject(new SystemModule.EagerSingletons()); c.inject(new TransportModule.EagerSingletons()); c.inject(new IntroductionModule.EagerSingletons()); } diff --git a/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java b/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java index 78c3cd7070..00f037a8c3 100644 --- a/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java +++ b/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java @@ -122,9 +122,6 @@ class PluginManagerImpl implements PluginManager, Service, EventListener { public void stopService() throws ServiceException { // Stop listening for events eventBus.removeListener(this); - // Stop the poller - LOG.info("Stopping poller"); - poller.stop(); final CountDownLatch latch = new CountDownLatch(plugins.size()); // Stop the simplex plugins LOG.info("Stopping simplex plugins"); @@ -432,7 +429,7 @@ class PluginManagerImpl implements PluginManager, Service, EventListener { public void transportEnabled() { eventBus.broadcast(new TransportEnabledEvent(id)); Plugin p = plugins.get(id); - if (p != null) poller.pollNow(p); + if (p != null && p.shouldPoll()) poller.pollNow(p); } @Override diff --git a/briar-core/src/org/briarproject/plugins/PluginsModule.java b/briar-core/src/org/briarproject/plugins/PluginsModule.java index 4129486659..97799fa24a 100644 --- a/briar-core/src/org/briarproject/plugins/PluginsModule.java +++ b/briar-core/src/org/briarproject/plugins/PluginsModule.java @@ -8,12 +8,10 @@ import org.briarproject.api.plugins.ConnectionManager; import org.briarproject.api.plugins.ConnectionRegistry; import org.briarproject.api.plugins.PluginManager; import org.briarproject.api.sync.SyncSessionFactory; -import org.briarproject.api.system.Timer; import org.briarproject.api.transport.KeyManager; import org.briarproject.api.transport.StreamReaderFactory; import org.briarproject.api.transport.StreamWriterFactory; -import java.security.SecureRandom; import java.util.concurrent.Executor; import javax.inject.Inject; @@ -36,10 +34,9 @@ public class PluginsModule { } @Provides - Poller providePoller(@IoExecutor Executor ioExecutor, - ConnectionRegistry connectionRegistry, SecureRandom random, - Timer timer) { - return new PollerImpl(ioExecutor, connectionRegistry, random, timer); + @Singleton + Poller providePoller(PollerImpl poller) { + return poller; } @Provides diff --git a/briar-core/src/org/briarproject/plugins/Poller.java b/briar-core/src/org/briarproject/plugins/Poller.java index 2cacf1561b..cb98150723 100644 --- a/briar-core/src/org/briarproject/plugins/Poller.java +++ b/briar-core/src/org/briarproject/plugins/Poller.java @@ -6,7 +6,4 @@ interface Poller { /** Tells the poller to poll the given plugin immediately. */ void pollNow(Plugin p); - - /** Stops the poller. */ - void stop(); } diff --git a/briar-core/src/org/briarproject/plugins/PollerImpl.java b/briar-core/src/org/briarproject/plugins/PollerImpl.java index db8e1b0b8d..843e455f63 100644 --- a/briar-core/src/org/briarproject/plugins/PollerImpl.java +++ b/briar-core/src/org/briarproject/plugins/PollerImpl.java @@ -4,17 +4,17 @@ import org.briarproject.api.TransportId; import org.briarproject.api.lifecycle.IoExecutor; import org.briarproject.api.plugins.ConnectionRegistry; import org.briarproject.api.plugins.Plugin; -import org.briarproject.api.system.Timer; import java.security.SecureRandom; import java.util.Map; -import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; import java.util.logging.Logger; import javax.inject.Inject; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.logging.Level.INFO; class PollerImpl implements Poller { @@ -23,31 +23,26 @@ class PollerImpl implements Poller { Logger.getLogger(PollerImpl.class.getName()); private final Executor ioExecutor; + private final ScheduledExecutorService scheduler; private final ConnectionRegistry connectionRegistry; private final SecureRandom random; - private final Timer timer; private final Map<TransportId, PollTask> tasks; @Inject PollerImpl(@IoExecutor Executor ioExecutor, - ConnectionRegistry connectionRegistry, SecureRandom random, - Timer timer) { + ScheduledExecutorService scheduler, + ConnectionRegistry connectionRegistry, SecureRandom random) { this.ioExecutor = ioExecutor; this.connectionRegistry = connectionRegistry; this.random = random; - this.timer = timer; + this.scheduler = scheduler; tasks = new ConcurrentHashMap<TransportId, PollTask>(); } - @Override - public void stop() { - timer.cancel(); - } - @Override public void pollNow(Plugin p) { // Randomise next polling interval - if (p.shouldPoll()) schedule(p, 0, true); + schedule(p, 0, true); } private void schedule(Plugin p, int interval, boolean randomiseNext) { @@ -55,7 +50,7 @@ class PollerImpl implements Poller { PollTask task = new PollTask(p, randomiseNext); PollTask replaced = tasks.put(p.getId(), task); if (replaced != null) replaced.cancel(); - timer.schedule(task, interval); + scheduler.schedule(task, interval, MILLISECONDS); } private void poll(final Plugin p) { @@ -69,18 +64,25 @@ class PollerImpl implements Poller { }); } - private class PollTask extends TimerTask { + private class PollTask implements Runnable { private final Plugin plugin; private final boolean randomiseNext; + private volatile boolean cancelled = false; + private PollTask(Plugin plugin, boolean randomiseNext) { this.plugin = plugin; this.randomiseNext = randomiseNext; } + private void cancel() { + cancelled = true; + } + @Override public void run() { + if (cancelled) return; tasks.remove(plugin.getId()); int interval = plugin.getPollingInterval(); if (randomiseNext) diff --git a/briar-core/src/org/briarproject/system/SystemModule.java b/briar-core/src/org/briarproject/system/SystemModule.java index eb2a0d88d2..8893a86ab0 100644 --- a/briar-core/src/org/briarproject/system/SystemModule.java +++ b/briar-core/src/org/briarproject/system/SystemModule.java @@ -1,23 +1,41 @@ package org.briarproject.system; +import org.briarproject.api.lifecycle.LifecycleManager; import org.briarproject.api.system.Clock; -import org.briarproject.api.system.LocationUtils; -import org.briarproject.api.system.SeedProvider; -import org.briarproject.api.system.Timer; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +import javax.inject.Inject; +import javax.inject.Singleton; import dagger.Module; import dagger.Provides; @Module public class SystemModule { + + public static class EagerSingletons { + @Inject + ScheduledExecutorService scheduledExecutorService; + } + + private final ScheduledExecutorService scheduler; + + public SystemModule() { + scheduler = Executors.newSingleThreadScheduledExecutor(); + } + @Provides Clock provideClock() { return new SystemClock(); } @Provides - Timer provideTimer() { - return new SystemTimer(); + @Singleton + ScheduledExecutorService provideScheduledExecutorService( + LifecycleManager lifecycleManager) { + lifecycleManager.registerForShutdown(scheduler); + return scheduler; } - } diff --git a/briar-core/src/org/briarproject/system/SystemTimer.java b/briar-core/src/org/briarproject/system/SystemTimer.java deleted file mode 100644 index fae6abbc75..0000000000 --- a/briar-core/src/org/briarproject/system/SystemTimer.java +++ /dev/null @@ -1,31 +0,0 @@ -package org.briarproject.system; - -import java.util.TimerTask; - -import org.briarproject.api.system.Timer; - -/** Default timer implementation. */ -public class SystemTimer implements Timer { - - private final java.util.Timer timer = new java.util.Timer(true); - - public void cancel() { - timer.cancel(); - } - - public int purge() { - return timer.purge(); - } - - public void schedule(TimerTask task, long delay) { - timer.schedule(task, delay); - } - - public void schedule(TimerTask task, long delay, long period) { - timer.schedule(task, delay, period); - } - - public void scheduleAtFixedRate(TimerTask task, long delay, long period) { - timer.scheduleAtFixedRate(task, delay, period); - } -} diff --git a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java index 590343ca2c..fdcbe8f30c 100644 --- a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java +++ b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java @@ -19,7 +19,6 @@ import org.briarproject.api.plugins.PluginConfig; import org.briarproject.api.plugins.duplex.DuplexPluginFactory; import org.briarproject.api.plugins.simplex.SimplexPluginFactory; import org.briarproject.api.system.Clock; -import org.briarproject.api.system.Timer; import org.briarproject.api.transport.KeyManager; import org.briarproject.api.transport.StreamContext; @@ -28,6 +27,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; import java.util.logging.Logger; import javax.inject.Inject; @@ -42,21 +42,22 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { private final DatabaseComponent db; private final CryptoComponent crypto; private final Executor dbExecutor; + private final ScheduledExecutorService scheduler; private final PluginConfig pluginConfig; - private final Timer timer; private final Clock clock; private final Map<ContactId, Boolean> activeContacts; private final ConcurrentHashMap<TransportId, TransportKeyManager> managers; @Inject KeyManagerImpl(DatabaseComponent db, CryptoComponent crypto, - @DatabaseExecutor Executor dbExecutor, PluginConfig pluginConfig, - Timer timer, Clock clock) { + @DatabaseExecutor Executor dbExecutor, + ScheduledExecutorService scheduler, PluginConfig pluginConfig, + Clock clock) { this.db = db; this.crypto = crypto; this.dbExecutor = dbExecutor; + this.scheduler = scheduler; this.pluginConfig = pluginConfig; - this.timer = timer; this.clock = clock; // Use a ConcurrentHashMap as a thread-safe set activeContacts = new ConcurrentHashMap<ContactId, Boolean>(); @@ -80,7 +81,8 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { db.addTransport(txn, e.getKey(), e.getValue()); for (Entry<TransportId, Integer> e : transports.entrySet()) { TransportKeyManager m = new TransportKeyManager(db, crypto, - timer, clock, e.getKey(), e.getValue()); + dbExecutor, scheduler, clock, e.getKey(), + e.getValue()); managers.put(e.getKey(), m); m.start(txn); } @@ -97,12 +99,14 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { public void stopService() { } + @Override public void addContact(Transaction txn, ContactId c, SecretKey master, long timestamp, boolean alice) throws DbException { for (TransportKeyManager m : managers.values()) m.addContact(txn, c, master, timestamp, alice); } + @Override public StreamContext getStreamContext(ContactId c, TransportId t) throws DbException { // Don't allow outgoing streams to inactive contacts @@ -123,6 +127,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { return ctx; } + @Override public StreamContext getStreamContext(TransportId t, byte[] tag) throws DbException { TransportKeyManager m = managers.get(t); @@ -141,6 +146,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { return ctx; } + @Override public void eventOccurred(Event e) { if (e instanceof ContactRemovedEvent) { removeContact(((ContactRemovedEvent) e).getContactId()); @@ -154,6 +160,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { private void removeContact(final ContactId c) { activeContacts.remove(c); dbExecutor.execute(new Runnable() { + @Override public void run() { for (TransportKeyManager m : managers.values()) m.removeContact(c); diff --git a/briar-core/src/org/briarproject/transport/TransportKeyManager.java b/briar-core/src/org/briarproject/transport/TransportKeyManager.java index 0c9b95bf46..1fb37877fc 100644 --- a/briar-core/src/org/briarproject/transport/TransportKeyManager.java +++ b/briar-core/src/org/briarproject/transport/TransportKeyManager.java @@ -9,7 +9,6 @@ import org.briarproject.api.db.DatabaseComponent; import org.briarproject.api.db.DbException; import org.briarproject.api.db.Transaction; import org.briarproject.api.system.Clock; -import org.briarproject.api.system.Timer; import org.briarproject.api.transport.StreamContext; import org.briarproject.api.transport.TransportKeys; import org.briarproject.transport.ReorderingWindow.Change; @@ -18,10 +17,12 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; -import java.util.TimerTask; +import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.logging.Level.WARNING; import static org.briarproject.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; import static org.briarproject.api.transport.TransportConstants.TAG_LENGTH; @@ -34,7 +35,8 @@ class TransportKeyManager { private final DatabaseComponent db; private final CryptoComponent crypto; - private final Timer timer; + private final Executor dbExecutor; + private final ScheduledExecutorService scheduler; private final Clock clock; private final TransportId transportId; private final long rotationPeriodLength; @@ -46,11 +48,12 @@ class TransportKeyManager { private final Map<ContactId, MutableTransportKeys> keys; TransportKeyManager(DatabaseComponent db, CryptoComponent crypto, - Timer timer, Clock clock, TransportId transportId, - long maxLatency) { + Executor dbExecutor, ScheduledExecutorService scheduler, + Clock clock, TransportId transportId, long maxLatency) { this.db = db; this.crypto = crypto; - this.timer = timer; + this.dbExecutor = dbExecutor; + this.scheduler = scheduler; this.clock = clock; this.transportId = transportId; rotationPeriodLength = maxLatency + MAX_CLOCK_DIFFERENCE; @@ -122,7 +125,19 @@ class TransportKeyManager { } private void scheduleKeyRotation(long now) { - TimerTask task = new TimerTask() { + Runnable task = new Runnable() { + @Override + public void run() { + rotateKeys(); + } + }; + long delay = rotationPeriodLength - now % rotationPeriodLength; + scheduler.schedule(task, delay, MILLISECONDS); + } + + private void rotateKeys() { + dbExecutor.execute(new Runnable() { + @Override public void run() { try { Transaction txn = db.startTransaction(false); @@ -137,9 +152,7 @@ class TransportKeyManager { LOG.log(WARNING, e.toString(), e); } } - }; - long delay = rotationPeriodLength - now % rotationPeriodLength; - timer.schedule(task, delay); + }); } void addContact(Transaction txn, ContactId c, SecretKey master, diff --git a/briar-core/src/org/briarproject/transport/TransportModule.java b/briar-core/src/org/briarproject/transport/TransportModule.java index 5cc81e458a..4c614ddc69 100644 --- a/briar-core/src/org/briarproject/transport/TransportModule.java +++ b/briar-core/src/org/briarproject/transport/TransportModule.java @@ -18,7 +18,8 @@ import dagger.Provides; public class TransportModule { public static class EagerSingletons { - @Inject KeyManager keyManager; + @Inject + KeyManager keyManager; } @Provides @@ -35,7 +36,7 @@ public class TransportModule { @Provides @Singleton - KeyManager getKeyManager(LifecycleManager lifecycleManager, + KeyManager provideKeyManager(LifecycleManager lifecycleManager, EventBus eventBus, KeyManagerImpl keyManager) { lifecycleManager.registerService(keyManager); eventBus.addListener(keyManager); diff --git a/briar-desktop/src/org/briarproject/system/DesktopSeedProviderModule.java b/briar-desktop/src/org/briarproject/system/DesktopSeedProviderModule.java new file mode 100644 index 0000000000..8ed175a8b2 --- /dev/null +++ b/briar-desktop/src/org/briarproject/system/DesktopSeedProviderModule.java @@ -0,0 +1,19 @@ +package org.briarproject.system; + +import org.briarproject.api.system.SeedProvider; +import org.briarproject.util.OsUtils; + +import javax.inject.Singleton; + +import dagger.Module; +import dagger.Provides; + +@Module +public class DesktopSeedProviderModule { + + @Provides + @Singleton + SeedProvider provideSeedProvider() { + return OsUtils.isLinux() ? new LinuxSeedProvider() : null; + } +} diff --git a/briar-desktop/src/org/briarproject/system/DesktopSystemModule.java b/briar-desktop/src/org/briarproject/system/DesktopSystemModule.java deleted file mode 100644 index ab811ebba8..0000000000 --- a/briar-desktop/src/org/briarproject/system/DesktopSystemModule.java +++ /dev/null @@ -1,31 +0,0 @@ -package org.briarproject.system; - -import org.briarproject.api.system.Clock; -import org.briarproject.api.system.SeedProvider; -import org.briarproject.api.system.Timer; -import org.briarproject.util.OsUtils; - -import dagger.Module; -import dagger.Provides; - -@Module -public class DesktopSystemModule { - - @Provides - Clock provideClock() { - return new SystemClock(); - } - - @Provides - Timer provideTimer() { - return new SystemTimer(); - } - - @Provides - SeedProvider provideSeedProvider() { - if (OsUtils.isLinux()) { - return new LinuxSeedProvider(); - } - return null; - } -} diff --git a/briar-tests/src/org/briarproject/TestSeedProviderModule.java b/briar-tests/src/org/briarproject/TestSeedProviderModule.java new file mode 100644 index 0000000000..a239ba7d8d --- /dev/null +++ b/briar-tests/src/org/briarproject/TestSeedProviderModule.java @@ -0,0 +1,18 @@ +package org.briarproject; + +import org.briarproject.api.system.SeedProvider; + +import javax.inject.Singleton; + +import dagger.Module; +import dagger.Provides; + +@Module +public class TestSeedProviderModule { + + @Provides + @Singleton + SeedProvider provideSeedProvider() { + return new TestSeedProvider(); + } +} diff --git a/briar-tests/src/org/briarproject/TestSystemModule.java b/briar-tests/src/org/briarproject/TestSystemModule.java deleted file mode 100644 index 2082896a31..0000000000 --- a/briar-tests/src/org/briarproject/TestSystemModule.java +++ /dev/null @@ -1,29 +0,0 @@ -package org.briarproject; - -import org.briarproject.api.system.Clock; -import org.briarproject.api.system.SeedProvider; -import org.briarproject.api.system.Timer; -import org.briarproject.system.SystemClock; -import org.briarproject.system.SystemTimer; - -import dagger.Module; -import dagger.Provides; - -@Module -public class TestSystemModule { - - @Provides - Clock provideClock() { - return new SystemClock(); - } - - @Provides - Timer provideSystemTimer() { - return new SystemTimer(); - } - - @Provides - SeedProvider provideSeedProvider() { - return new TestSeedProvider(); - } -} diff --git a/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java b/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java index cd2400c778..9f094dadbc 100644 --- a/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java +++ b/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java @@ -113,8 +113,6 @@ public class PluginManagerImplTest extends BriarTestCase { // stop() // Stop listening for events oneOf(eventBus).removeListener(with(any(EventListener.class))); - // Stop the poller - oneOf(poller).stop(); // Stop the plugins oneOf(simplexPlugin).stop(); oneOf(duplexPlugin).stop(); @@ -248,8 +246,6 @@ public class PluginManagerImplTest extends BriarTestCase { // stop() // Stop listening for events oneOf(eventBus).removeListener(with(any(EventListener.class))); - // Stop the poller - oneOf(poller).stop(); // Stop the plugins oneOf(simplexPlugin).stop(); oneOf(simplexPlugin1).stop(); diff --git a/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java b/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java index 68df77e413..bf035fd3c7 100644 --- a/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java +++ b/briar-tests/src/org/briarproject/transport/TransportKeyManagerTest.java @@ -9,7 +9,6 @@ import org.briarproject.api.crypto.SecretKey; import org.briarproject.api.db.DatabaseComponent; import org.briarproject.api.db.Transaction; import org.briarproject.api.system.Clock; -import org.briarproject.api.system.Timer; import org.briarproject.api.transport.IncomingKeys; import org.briarproject.api.transport.OutgoingKeys; import org.briarproject.api.transport.StreamContext; @@ -28,8 +27,10 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Random; -import java.util.TimerTask; +import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.briarproject.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; import static org.briarproject.api.transport.TransportConstants.REORDERING_WINDOW_SIZE; import static org.briarproject.api.transport.TransportConstants.TAG_LENGTH; @@ -56,11 +57,12 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); - final Map<ContactId, TransportKeys> loaded = - new LinkedHashMap<ContactId, TransportKeys>(); + final Map<ContactId, TransportKeys> loaded = new LinkedHashMap<>(); final TransportKeys shouldRotate = createTransportKeys(900, 0); final TransportKeys shouldNotRotate = createTransportKeys(1000, 0); loaded.put(contactId, shouldRotate); @@ -90,12 +92,12 @@ public class TransportKeyManagerTest extends BriarTestCase { oneOf(db).updateTransportKeys(txn, Collections.singletonMap(contactId, rotated)); // Schedule key rotation at the start of the next rotation period - oneOf(timer).schedule(with(any(TimerTask.class)), - with(rotationPeriodLength - 1)); + oneOf(scheduler).schedule(with(any(Runnable.class)), + with(rotationPeriodLength - 1), with(MILLISECONDS)); }}); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); transportKeyManager.start(txn); context.assertIsSatisfied(); @@ -106,7 +108,9 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); final boolean alice = true; @@ -135,7 +139,7 @@ public class TransportKeyManagerTest extends BriarTestCase { }}); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); // The timestamp is 1 ms before the start of rotation period 1000 long timestamp = rotationPeriodLength * 1000 - 1; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, @@ -150,13 +154,15 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); final Transaction txn = new Transaction(null, false); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); assertNull(transportKeyManager.getStreamContext(txn, contactId)); context.assertIsSatisfied(); @@ -168,7 +174,9 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); final boolean alice = true; @@ -198,7 +206,7 @@ public class TransportKeyManagerTest extends BriarTestCase { }}); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); // The timestamp is at the start of rotation period 1000 long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, @@ -213,7 +221,9 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); final boolean alice = true; @@ -245,7 +255,7 @@ public class TransportKeyManagerTest extends BriarTestCase { }}); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); // The timestamp is at the start of rotation period 1000 long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, @@ -271,7 +281,9 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); final boolean alice = true; @@ -299,7 +311,7 @@ public class TransportKeyManagerTest extends BriarTestCase { }}); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); // The timestamp is at the start of rotation period 1000 long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, @@ -315,13 +327,15 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); final boolean alice = true; final TransportKeys transportKeys = createTransportKeys(1000, 0); // Keep a copy of the tags - final List<byte[]> tags = new ArrayList<byte[]>(); + final List<byte[]> tags = new ArrayList<>(); final Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ @@ -352,7 +366,7 @@ public class TransportKeyManagerTest extends BriarTestCase { }}); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); // The timestamp is at the start of rotation period 1000 long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, @@ -381,7 +395,9 @@ public class TransportKeyManagerTest extends BriarTestCase { Mockery context = new Mockery(); final DatabaseComponent db = context.mock(DatabaseComponent.class); final CryptoComponent crypto = context.mock(CryptoComponent.class); - final Timer timer = context.mock(Timer.class); + final Executor dbExecutor = context.mock(Executor.class); + final ScheduledExecutorService scheduler = + context.mock(ScheduledExecutorService.class); final Clock clock = context.mock(Clock.class); final TransportKeys transportKeys = createTransportKeys(1000, 0); @@ -408,9 +424,11 @@ public class TransportKeyManagerTest extends BriarTestCase { will(new EncodeTagAction()); } // Schedule key rotation at the start of the next rotation period - oneOf(timer).schedule(with(any(TimerTask.class)), - with(rotationPeriodLength)); - will(new RunTimerTaskAction()); + oneOf(scheduler).schedule(with(any(Runnable.class)), + with(rotationPeriodLength), with(MILLISECONDS)); + will(new RunAction()); + oneOf(dbExecutor).execute(with(any(Runnable.class))); + will(new RunAction()); // Start a transaction for key rotation oneOf(db).startTransaction(false); will(returnValue(txn1)); @@ -431,14 +449,14 @@ public class TransportKeyManagerTest extends BriarTestCase { oneOf(db).updateTransportKeys(txn1, Collections.singletonMap(contactId, rotated)); // Schedule key rotation at the start of the next rotation period - oneOf(timer).schedule(with(any(TimerTask.class)), - with(rotationPeriodLength)); + oneOf(scheduler).schedule(with(any(Runnable.class)), + with(rotationPeriodLength), with(MILLISECONDS)); // Commit the key rotation transaction oneOf(db).endTransaction(txn1); }}); TransportKeyManager transportKeyManager = new TransportKeyManager(db, - crypto, timer, clock, transportId, maxLatency); + crypto, dbExecutor, scheduler, clock, transportId, maxLatency); transportKeyManager.start(txn); assertTrue(txn1.isComplete()); @@ -484,18 +502,18 @@ public class TransportKeyManagerTest extends BriarTestCase { } } - private static class RunTimerTaskAction implements Action { + private static class RunAction implements Action { @Override public Object invoke(Invocation invocation) throws Throwable { - TimerTask task = (TimerTask) invocation.getParameter(0); + Runnable task = (Runnable) invocation.getParameter(0); task.run(); return null; } @Override public void describeTo(Description description) { - description.appendText("schedules a timer task"); + description.appendText("runs a runnable"); } } } -- GitLab