From 3aa00ecb3d4a08c32132c58da69d0fff593e9151 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 6 Aug 2020 13:11:12 +0100 Subject: [PATCH 01/11] Pass executor to scheduler. --- .../network/AndroidNetworkManager.java | 9 +++-- .../bramble/system/AndroidTaskScheduler.java | 19 ++++++----- .../system/AndroidWakeLockFactoryImpl.java | 10 ++++-- .../bramble/system/RenewableWakeLock.java | 18 +++++++--- .../bramble/api/system/TaskScheduler.java | 15 +++++---- .../bramble/io/TimeoutMonitorImpl.java | 31 +++++++++-------- .../bramble/plugin/PollerImpl.java | 9 ++--- .../rendezvous/RendezvousPollerImpl.java | 12 +++---- .../bramble/system/TaskSchedulerImpl.java | 17 ++++++---- .../transport/TransportKeyManagerImpl.java | 16 ++++----- .../bramble/plugin/PollerImplTest.java | 33 +++++++++++-------- .../rendezvous/RendezvousPollerImplTest.java | 16 ++++----- .../TransportKeyManagerImplTest.java | 9 +++-- .../briar/feed/FeedManagerImpl.java | 6 ++-- 14 files changed, 128 insertions(+), 92 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/network/AndroidNetworkManager.java b/bramble-android/src/main/java/org/briarproject/bramble/network/AndroidNetworkManager.java index c113dca2b..f6c238600 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/network/AndroidNetworkManager.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/network/AndroidNetworkManager.java @@ -9,6 +9,7 @@ import android.net.ConnectivityManager; import android.net.NetworkInfo; import org.briarproject.bramble.api.event.EventBus; +import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.lifecycle.Service; import org.briarproject.bramble.api.network.NetworkManager; import org.briarproject.bramble.api.network.NetworkStatus; @@ -17,6 +18,7 @@ import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.system.TaskScheduler; +import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -51,6 +53,7 @@ class AndroidNetworkManager implements NetworkManager, Service { private final TaskScheduler scheduler; private final EventBus eventBus; + private final Executor eventExecutor; private final Context appContext; private final AtomicReference> connectivityCheck = new AtomicReference<>(); @@ -60,9 +63,10 @@ class AndroidNetworkManager implements NetworkManager, Service { @Inject AndroidNetworkManager(TaskScheduler scheduler, EventBus eventBus, - Application app) { + @EventExecutor Executor eventExecutor, Application app) { this.scheduler = scheduler; this.eventBus = eventBus; + this.eventExecutor = eventExecutor; this.appContext = app.getApplicationContext(); } @@ -104,7 +108,8 @@ class AndroidNetworkManager implements NetworkManager, Service { private void scheduleConnectionStatusUpdate(int delay, TimeUnit unit) { Future newConnectivityCheck = - scheduler.schedule(this::updateConnectionStatus, delay, unit); + scheduler.schedule(this::updateConnectionStatus, eventExecutor, + delay, unit); Future oldConnectivityCheck = connectivityCheck.getAndSet(newConnectivityCheck); if (oldConnectivityCheck != null) oldConnectivityCheck.cancel(false); diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java index 871f8217d..074f57632 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.List; import java.util.PriorityQueue; import java.util.Queue; +import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.concurrent.FutureTask; import java.util.concurrent.ScheduledExecutorService; @@ -78,10 +79,11 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { } @Override - public Future schedule(Runnable task, long delay, TimeUnit unit) { + public Future schedule(Runnable task, Executor executor, long delay, + TimeUnit unit) { long now = SystemClock.elapsedRealtime(); long dueMillis = now + MILLISECONDS.convert(delay, unit); - ScheduledTask s = new ScheduledTask(task, dueMillis); + ScheduledTask s = new ScheduledTask(task, executor, dueMillis); if (dueMillis <= now) { scheduledExecutorService.execute(s); } else { @@ -93,13 +95,13 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { } @Override - public Future scheduleWithFixedDelay(Runnable task, long delay, - long interval, TimeUnit unit) { + public Future scheduleWithFixedDelay(Runnable task, Executor executor, + long delay, long interval, TimeUnit unit) { Runnable wrapped = () -> { task.run(); - scheduleWithFixedDelay(task, interval, interval, unit); + scheduleWithFixedDelay(task, executor, interval, interval, unit); }; - return schedule(wrapped, delay, unit); + return schedule(wrapped, executor, delay, unit); } @Override @@ -175,8 +177,9 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { private final long dueMillis; - public ScheduledTask(Runnable runnable, long dueMillis) { - super(runnable, null); + public ScheduledTask(Runnable runnable, Executor executor, + long dueMillis) { + super(() -> executor.execute(runnable), null); this.dueMillis = dueMillis; } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockFactoryImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockFactoryImpl.java index e61af6644..01ee5f563 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockFactoryImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockFactoryImpl.java @@ -6,11 +6,14 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.os.PowerManager; +import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AndroidWakeLock; import org.briarproject.bramble.api.system.AndroidWakeLockFactory; import org.briarproject.bramble.api.system.TaskScheduler; +import java.util.concurrent.Executor; + import javax.annotation.concurrent.Immutable; import javax.inject.Inject; @@ -38,12 +41,15 @@ class AndroidWakeLockFactoryImpl implements AndroidWakeLockFactory { private final SharedWakeLock sharedWakeLock; @Inject - AndroidWakeLockFactoryImpl(TaskScheduler scheduler, Application app) { + AndroidWakeLockFactoryImpl(TaskScheduler scheduler, + @EventExecutor Executor eventExecutor, + Application app) { PowerManager powerManager = (PowerManager) requireNonNull(app.getSystemService(POWER_SERVICE)); String tag = getWakeLockTag(app); sharedWakeLock = new RenewableWakeLock(powerManager, scheduler, - PARTIAL_WAKE_LOCK, tag, LOCK_DURATION_MS, SAFETY_MARGIN_MS); + eventExecutor, PARTIAL_WAKE_LOCK, tag, LOCK_DURATION_MS, + SAFETY_MARGIN_MS); } @Override diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java b/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java index c47a8dfb9..a2852c17e 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java @@ -6,6 +6,7 @@ import android.os.PowerManager.WakeLock; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.TaskScheduler; +import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.logging.Logger; @@ -28,6 +29,7 @@ class RenewableWakeLock implements SharedWakeLock { private final PowerManager powerManager; private final TaskScheduler scheduler; + private final Executor eventExecutor; private final int levelAndFlags; private final String tag; private final long durationMs, safetyMarginMs; @@ -44,11 +46,16 @@ class RenewableWakeLock implements SharedWakeLock { @GuardedBy("lock") private long acquired = 0; - RenewableWakeLock(PowerManager powerManager, TaskScheduler scheduler, - int levelAndFlags, String tag, long durationMs, + RenewableWakeLock(PowerManager powerManager, + TaskScheduler scheduler, + Executor eventExecutor, + int levelAndFlags, + String tag, + long durationMs, long safetyMarginMs) { this.powerManager = powerManager; this.scheduler = scheduler; + this.eventExecutor = eventExecutor; this.levelAndFlags = levelAndFlags; this.tag = tag; this.durationMs = durationMs; @@ -69,8 +76,8 @@ class RenewableWakeLock implements SharedWakeLock { // power management apps wakeLock.setReferenceCounted(false); wakeLock.acquire(durationMs + safetyMarginMs); - future = scheduler.schedule(this::renew, durationMs, - MILLISECONDS); + future = scheduler.schedule(this::renew, eventExecutor, + durationMs, MILLISECONDS); acquired = android.os.SystemClock.elapsedRealtime(); } } @@ -93,7 +100,8 @@ class RenewableWakeLock implements SharedWakeLock { wakeLock.setReferenceCounted(false); wakeLock.acquire(durationMs + safetyMarginMs); oldWakeLock.release(); - future = scheduler.schedule(this::renew, durationMs, MILLISECONDS); + future = scheduler.schedule(this::renew, eventExecutor, + durationMs, MILLISECONDS); acquired = now; } } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java b/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java index dad4ef86c..96c977dd4 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java @@ -2,8 +2,8 @@ package org.briarproject.bramble.api.system; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import java.util.concurrent.Executor; import java.util.concurrent.Future; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; /** @@ -16,13 +16,16 @@ import java.util.concurrent.TimeUnit; public interface TaskScheduler { /** - * See {@link ScheduledExecutorService#schedule(Runnable, long, TimeUnit)}. + * Submits the given task to the given executor after the given delay. */ - Future schedule(Runnable task, long delay, TimeUnit unit); + Future schedule(Runnable task, Executor executor, long delay, + TimeUnit unit); /** - * See {@link ScheduledExecutorService#scheduleWithFixedDelay(Runnable, long, long, TimeUnit)}. + * Submits the given task to the given executor after the given delay, + * and then repeatedly with the given interval between executions + * (measured from the end of one execution to the beginning of the next). */ - Future scheduleWithFixedDelay(Runnable task, long delay, - long interval, TimeUnit unit); + Future scheduleWithFixedDelay(Runnable task, Executor executor, + long delay, long interval, TimeUnit unit); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/io/TimeoutMonitorImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/io/TimeoutMonitorImpl.java index ce90af8f0..e4bc71e1d 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/io/TimeoutMonitorImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/io/TimeoutMonitorImpl.java @@ -55,7 +55,8 @@ class TimeoutMonitorImpl implements TimeoutMonitor { synchronized (lock) { if (streams.isEmpty()) { task = scheduler.scheduleWithFixedDelay(this::checkTimeouts, - CHECK_INTERVAL_MS, CHECK_INTERVAL_MS, MILLISECONDS); + ioExecutor, CHECK_INTERVAL_MS, CHECK_INTERVAL_MS, + MILLISECONDS); } streams.add(stream); } @@ -73,23 +74,21 @@ class TimeoutMonitorImpl implements TimeoutMonitor { if (toCancel != null) toCancel.cancel(false); } - // Scheduler + @IoExecutor private void checkTimeouts() { - ioExecutor.execute(() -> { - List snapshot; - synchronized (lock) { - snapshot = new ArrayList<>(streams); - } - for (TimeoutInputStream stream : snapshot) { - if (stream.hasTimedOut()) { - LOG.info("Input stream has timed out"); - try { - stream.close(); - } catch (IOException e) { - logException(LOG, INFO, e); - } + List snapshot; + synchronized (lock) { + snapshot = new ArrayList<>(streams); + } + for (TimeoutInputStream stream : snapshot) { + if (stream.hasTimedOut()) { + LOG.info("Input stream has timed out"); + try { + stream.close(); + } catch (IOException e) { + logException(LOG, INFO, e); } } - }); + } } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java index db5a878a9..361ac244b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java @@ -118,6 +118,7 @@ class PollerImpl implements Poller, EventListener { } } + // TODO: Make this wakeful private void connectToContact(ContactId c) { for (SimplexPlugin s : pluginManager.getSimplexPlugins()) if (s.shouldPoll()) connectToContact(c, s); @@ -189,8 +190,8 @@ class PollerImpl implements Poller, EventListener { // it will abort safely when it finds it's been replaced if (scheduled != null) scheduled.future.cancel(false); PollTask task = new PollTask(p, due, randomiseNext); - Future future = scheduler.schedule(() -> - ioExecutor.execute(task), delay, MILLISECONDS); + Future future = scheduler.schedule(task, ioExecutor, delay, + MILLISECONDS); tasks.put(t, new ScheduledPollTask(task, future)); } } finally { @@ -233,9 +234,9 @@ class PollerImpl implements Poller, EventListener { private class ScheduledPollTask { private final PollTask task; - private final Future future; + private final Future future; - private ScheduledPollTask(PollTask task, Future future) { + private ScheduledPollTask(PollTask task, Future future) { this.task = task; this.future = future; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/rendezvous/RendezvousPollerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/rendezvous/RendezvousPollerImpl.java index 82bd71a46..22f781617 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/rendezvous/RendezvousPollerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/rendezvous/RendezvousPollerImpl.java @@ -143,8 +143,8 @@ class RendezvousPollerImpl implements RendezvousPoller, Service, EventListener { } catch (DbException e) { throw new ServiceException(e); } - scheduler.scheduleWithFixedDelay(this::poll, POLLING_INTERVAL_MS, - POLLING_INTERVAL_MS, MILLISECONDS); + scheduler.scheduleWithFixedDelay(this::poll, worker, + POLLING_INTERVAL_MS, POLLING_INTERVAL_MS, MILLISECONDS); } @EventExecutor @@ -204,12 +204,10 @@ class RendezvousPollerImpl implements RendezvousPoller, Service, EventListener { return plugin.createRendezvousEndpoint(k, cs.alice, h); } - // Scheduler + // Worker private void poll() { - worker.execute(() -> { - removeExpiredPendingContacts(); - for (PluginState ps : pluginStates.values()) poll(ps); - }); + removeExpiredPendingContacts(); + for (PluginState ps : pluginStates.values()) poll(ps); } // Worker diff --git a/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java index 34af175b3..1627cad0f 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java @@ -3,6 +3,7 @@ package org.briarproject.bramble.system; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.TaskScheduler; +import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -10,8 +11,7 @@ import java.util.concurrent.TimeUnit; import javax.annotation.concurrent.ThreadSafe; /** - * A {@link TaskScheduler} that delegates all calls to a - * {@link ScheduledExecutorService}. + * A {@link TaskScheduler} that uses a {@link ScheduledExecutorService}. */ @ThreadSafe @NotNullByDefault @@ -24,13 +24,16 @@ class TaskSchedulerImpl implements TaskScheduler { } @Override - public Future schedule(Runnable task, long delay, TimeUnit unit) { - return delegate.schedule(task, delay, unit); + public Future schedule(Runnable task, Executor executor, long delay, + TimeUnit unit) { + Runnable execute = () -> executor.execute(task); + return delegate.schedule(execute, delay, unit); } @Override - public Future scheduleWithFixedDelay(Runnable task, long delay, - long interval, TimeUnit unit) { - return delegate.scheduleWithFixedDelay(task, delay, interval, unit); + public Future scheduleWithFixedDelay(Runnable task, Executor executor, + long delay, long interval, TimeUnit unit) { + Runnable execute = () -> executor.execute(task); + return delegate.scheduleWithFixedDelay(execute, delay, interval, unit); } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java index b255e196f..f15fd82c7 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java @@ -6,6 +6,7 @@ import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.crypto.TransportCrypto; import org.briarproject.bramble.api.db.DatabaseComponent; +import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @@ -198,17 +199,16 @@ class TransportKeyManagerImpl implements TransportKeyManager { private void scheduleKeyUpdate(long now) { long delay = timePeriodLength - now % timePeriodLength; - scheduler.schedule(this::updateKeys, delay, MILLISECONDS); + scheduler.schedule(this::updateKeys, dbExecutor, delay, MILLISECONDS); } + @DatabaseExecutor private void updateKeys() { - dbExecutor.execute(() -> { - try { - db.transaction(false, this::updateKeys); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); + try { + db.transaction(false, this::updateKeys); + } catch (DbException e) { + logException(LOG, WARNING, e); + } } @Override diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java index 04cd467bd..99fb4b4ec 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java @@ -234,7 +234,8 @@ public class PollerImplTest extends BrambleMockTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(now)); oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) pollingInterval), with(MILLISECONDS)); + with(ioExecutor), with((long) pollingInterval), + with(MILLISECONDS)); will(returnValue(future)); }}); @@ -262,7 +263,8 @@ public class PollerImplTest extends BrambleMockTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(now)); oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) pollingInterval), with(MILLISECONDS)); + with(ioExecutor), with((long) pollingInterval), + with(MILLISECONDS)); will(returnValue(future)); // Second event // Get the plugin @@ -304,7 +306,8 @@ public class PollerImplTest extends BrambleMockTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(now)); oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) pollingInterval), with(MILLISECONDS)); + with(ioExecutor), with((long) pollingInterval), + with(MILLISECONDS)); will(returnValue(future)); // Second event // Get the plugin @@ -320,7 +323,8 @@ public class PollerImplTest extends BrambleMockTestCase { will(returnValue(now + 1)); oneOf(future).cancel(false); oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) pollingInterval - 2), with(MILLISECONDS)); + with(ioExecutor), with((long) pollingInterval - 2), + with(MILLISECONDS)); }}); poller.eventOccurred(new ConnectionOpenedEvent(contactId, transportId, @@ -345,8 +349,8 @@ public class PollerImplTest extends BrambleMockTestCase { // Schedule a polling task immediately oneOf(clock).currentTimeMillis(); will(returnValue(now)); - oneOf(scheduler).schedule(with(any(Runnable.class)), with(0L), - with(MILLISECONDS)); + oneOf(scheduler).schedule(with(any(Runnable.class)), + with(ioExecutor), with(0L), with(MILLISECONDS)); will(returnValue(future)); will(new RunAction()); // Running the polling task schedules the next polling task @@ -357,7 +361,8 @@ public class PollerImplTest extends BrambleMockTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(now)); oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) (pollingInterval * 0.5)), with(MILLISECONDS)); + with(ioExecutor), with((long) (pollingInterval * 0.5)), + with(MILLISECONDS)); will(returnValue(future)); // Get the transport properties and connected contacts oneOf(transportPropertyManager).getRemoteProperties(transportId); @@ -388,8 +393,8 @@ public class PollerImplTest extends BrambleMockTestCase { // Schedule a polling task immediately oneOf(clock).currentTimeMillis(); will(returnValue(now)); - oneOf(scheduler).schedule(with(any(Runnable.class)), with(0L), - with(MILLISECONDS)); + oneOf(scheduler).schedule(with(any(Runnable.class)), + with(ioExecutor), with(0L), with(MILLISECONDS)); will(returnValue(future)); will(new RunAction()); // Running the polling task schedules the next polling task @@ -400,7 +405,8 @@ public class PollerImplTest extends BrambleMockTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(now)); oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) (pollingInterval * 0.5)), with(MILLISECONDS)); + with(ioExecutor), with((long) (pollingInterval * 0.5)), + with(MILLISECONDS)); will(returnValue(future)); // Get the transport properties and connected contacts oneOf(transportPropertyManager).getRemoteProperties(transportId); @@ -429,8 +435,8 @@ public class PollerImplTest extends BrambleMockTestCase { // Schedule a polling task immediately oneOf(clock).currentTimeMillis(); will(returnValue(now)); - oneOf(scheduler).schedule(with(any(Runnable.class)), with(0L), - with(MILLISECONDS)); + oneOf(scheduler).schedule(with(any(Runnable.class)), + with(ioExecutor), with(0L), with(MILLISECONDS)); will(returnValue(future)); // The plugin is deactivated before the task runs - cancel the task oneOf(future).cancel(false); @@ -454,7 +460,8 @@ public class PollerImplTest extends BrambleMockTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(now)); oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) pollingInterval), with(MILLISECONDS)); + with(ioExecutor), with((long) pollingInterval), + with(MILLISECONDS)); will(returnValue(future)); }}); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java index c7238cec3..2945455c5 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/rendezvous/RendezvousPollerImplTest.java @@ -123,8 +123,8 @@ public class RendezvousPollerImplTest extends BrambleMockTestCase { e.getPendingContactState() == OFFLINE))); // Capture the poll task oneOf(scheduler).scheduleWithFixedDelay(with(any(Runnable.class)), - with(POLLING_INTERVAL_MS), with(POLLING_INTERVAL_MS), - with(MILLISECONDS)); + with(any(Executor.class)), with(POLLING_INTERVAL_MS), + with(POLLING_INTERVAL_MS), with(MILLISECONDS)); will(new CaptureArgumentAction<>(capturePollTask, Runnable.class, 0)); }}); @@ -159,8 +159,8 @@ public class RendezvousPollerImplTest extends BrambleMockTestCase { e.getPendingContactState() == FAILED))); // Schedule the poll task oneOf(scheduler).scheduleWithFixedDelay(with(any(Runnable.class)), - with(POLLING_INTERVAL_MS), with(POLLING_INTERVAL_MS), - with(MILLISECONDS)); + with(any(Executor.class)), with(POLLING_INTERVAL_MS), + with(POLLING_INTERVAL_MS), with(MILLISECONDS)); }}); rendezvousPoller.startService(); @@ -468,8 +468,8 @@ public class RendezvousPollerImplTest extends BrambleMockTestCase { will(returnValue(emptyList())); // Capture the poll task oneOf(scheduler).scheduleWithFixedDelay(with(any(Runnable.class)), - with(POLLING_INTERVAL_MS), with(POLLING_INTERVAL_MS), - with(MILLISECONDS)); + with(any(Executor.class)), with(POLLING_INTERVAL_MS), + with(POLLING_INTERVAL_MS), with(MILLISECONDS)); will(new CaptureArgumentAction<>(capturePollTask, Runnable.class, 0)); }}); @@ -545,8 +545,8 @@ public class RendezvousPollerImplTest extends BrambleMockTestCase { e.getPendingContactState() == OFFLINE))); // Capture the poll task oneOf(scheduler).scheduleWithFixedDelay(with(any(Runnable.class)), - with(POLLING_INTERVAL_MS), with(POLLING_INTERVAL_MS), - with(MILLISECONDS)); + with(any(Executor.class)), with(POLLING_INTERVAL_MS), + with(POLLING_INTERVAL_MS), with(MILLISECONDS)); will(new CaptureArgumentAction<>(capturePollTask, Runnable.class, 0)); }}); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java index 2c7140cd6..084e15463 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java @@ -117,7 +117,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { new TransportKeySet(keySetId, contactId, null, updated))); // Schedule a key update at the start of the next time period oneOf(scheduler).schedule(with(any(Runnable.class)), - with(timePeriodLength - 1), with(MILLISECONDS)); + with(dbExecutor), with(timePeriodLength - 1), + with(MILLISECONDS)); }}); transportKeyManager.start(txn); @@ -420,7 +421,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } // Schedule a key update at the start of the next time period oneOf(scheduler).schedule(with(any(Runnable.class)), - with(timePeriodLength), with(MILLISECONDS)); + with(dbExecutor), with(timePeriodLength), + with(MILLISECONDS)); will(new RunAction()); oneOf(dbExecutor).execute(with(any(Runnable.class))); will(new RunAction()); @@ -445,7 +447,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { new TransportKeySet(keySetId, contactId, null, updated))); // Schedule a key update at the start of the next time period oneOf(scheduler).schedule(with(any(Runnable.class)), - with(timePeriodLength), with(MILLISECONDS)); + with(dbExecutor), with(timePeriodLength), + with(MILLISECONDS)); }}); transportKeyManager.start(txn); diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index 2b21060ec..713e2c090 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -146,9 +146,8 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, private void startFeedExecutor() { if (fetcherStarted.getAndSet(true)) return; LOG.info("Tor started, scheduling RSS feed fetcher"); - Runnable fetcher = () -> ioExecutor.execute(this::fetchFeeds); - scheduler.scheduleWithFixedDelay(fetcher, FETCH_DELAY_INITIAL, - FETCH_INTERVAL, FETCH_UNIT); + scheduler.scheduleWithFixedDelay(this::fetchFeeds, ioExecutor, + FETCH_DELAY_INITIAL, FETCH_INTERVAL, FETCH_UNIT); } @Override @@ -471,6 +470,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, if (date == null) time = now; else time = Math.max(0, Math.min(date.getTime(), now)); String text = getPostText(b.toString()); + //noinspection TryWithIdenticalCatches try { // create and store post LocalAuthor localAuthor = feed.getLocalAuthor(); From 94dd0a266141a7464f8ed24b1e0c964ce3c00e75 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 6 Aug 2020 14:53:05 +0100 Subject: [PATCH 02/11] Hold a wake lock while scheduled tasks are running. --- .../bramble/system/AndroidTaskScheduler.java | 28 ++++++++++++++++--- .../system/AndroidTaskSchedulerModule.java | 8 ++++-- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java index 074f57632..3b8ade660 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java @@ -11,6 +11,8 @@ import android.os.SystemClock; import org.briarproject.bramble.api.lifecycle.Service; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AlarmListener; +import org.briarproject.bramble.api.system.AndroidWakeLock; +import org.briarproject.bramble.api.system.AndroidWakeLockFactory; import org.briarproject.bramble.api.system.TaskScheduler; import java.util.ArrayList; @@ -51,6 +53,7 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { private static final long ALARM_MS = INTERVAL_FIFTEEN_MINUTES; private final Application app; + private final AndroidWakeLockFactory wakeLockFactory; private final ScheduledExecutorService scheduledExecutorService; private final AlarmManager alarmManager; @@ -59,8 +62,10 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { private final Queue tasks = new PriorityQueue<>(); AndroidTaskScheduler(Application app, + AndroidWakeLockFactory wakeLockFactory, ScheduledExecutorService scheduledExecutorService) { this.app = app; + this.wakeLockFactory = wakeLockFactory; this.scheduledExecutorService = scheduledExecutorService; alarmManager = (AlarmManager) requireNonNull(app.getSystemService(ALARM_SERVICE)); @@ -83,7 +88,8 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { TimeUnit unit) { long now = SystemClock.elapsedRealtime(); long dueMillis = now + MILLISECONDS.convert(delay, unit); - ScheduledTask s = new ScheduledTask(task, executor, dueMillis); + Runnable wakeful = createWakefulTask(task, executor); + ScheduledTask s = new ScheduledTask(wakeful, dueMillis); if (dueMillis <= now) { scheduledExecutorService.execute(s); } else { @@ -118,6 +124,21 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { } } + private Runnable createWakefulTask(Runnable task, Executor executor) { + // Hold a wake lock from before we submit the task until after it runs + AndroidWakeLock wakeLock = wakeLockFactory.createWakeLock(); + return () -> { + wakeLock.acquire(); + executor.execute(() -> { + try { + task.run(); + } finally { + wakeLock.release(); + } + }); + }; + } + private void runDueTasks() { long now = SystemClock.elapsedRealtime(); List due = new ArrayList<>(); @@ -177,9 +198,8 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { private final long dueMillis; - public ScheduledTask(Runnable runnable, Executor executor, - long dueMillis) { - super(() -> executor.execute(runnable), null); + public ScheduledTask(Runnable runnable, long dueMillis) { + super(runnable, null); this.dueMillis = dueMillis; } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java index 88fc77ff9..fd47e7523 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java @@ -4,6 +4,7 @@ import android.app.Application; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.system.AlarmListener; +import org.briarproject.bramble.api.system.AndroidWakeLockFactory; import org.briarproject.bramble.api.system.TaskScheduler; import java.util.concurrent.RejectedExecutionHandler; @@ -36,10 +37,11 @@ public class AndroidTaskSchedulerModule { @Provides @Singleton AndroidTaskScheduler provideAndroidTaskScheduler( - LifecycleManager lifecycleManager, Application app) { + LifecycleManager lifecycleManager, Application app, + AndroidWakeLockFactory wakeLockFactory) { lifecycleManager.registerForShutdown(scheduledExecutorService); - AndroidTaskScheduler scheduler = - new AndroidTaskScheduler(app, scheduledExecutorService); + AndroidTaskScheduler scheduler = new AndroidTaskScheduler(app, + wakeLockFactory, scheduledExecutorService); lifecycleManager.registerService(scheduler); return scheduler; } From 942bb28701e675c967b0a3bf668e9d73b9d118bc Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 6 Aug 2020 15:09:01 +0100 Subject: [PATCH 03/11] Hold a wake lock while running due tasks. --- ...ctory.java => AndroidWakeLockManager.java} | 4 ++- .../AndroidBluetoothConnectionFactory.java | 10 +++--- .../AndroidBluetoothPluginFactory.java | 10 +++--- .../AndroidBluetoothTransportConnection.java | 6 ++-- .../bramble/plugin/tor/AndroidTorPlugin.java | 6 ++-- .../plugin/tor/AndroidTorPluginFactory.java | 10 +++--- .../bramble/system/AndroidSystemModule.java | 8 ++--- .../bramble/system/AndroidTaskScheduler.java | 35 ++++++++++--------- .../system/AndroidTaskSchedulerModule.java | 6 ++-- ...l.java => AndroidWakeLockManagerImpl.java} | 17 +++++++-- .../briarproject/briar/android/AppModule.java | 8 ++--- 11 files changed, 68 insertions(+), 52 deletions(-) rename bramble-android/src/main/java/org/briarproject/bramble/api/system/{AndroidWakeLockFactory.java => AndroidWakeLockManager.java} (69%) rename bramble-android/src/main/java/org/briarproject/bramble/system/{AndroidWakeLockFactoryImpl.java => AndroidWakeLockManagerImpl.java} (85%) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockFactory.java b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java similarity index 69% rename from bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockFactory.java rename to bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java index 7d7be15d0..ba6c3ebc3 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockFactory.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java @@ -3,7 +3,9 @@ package org.briarproject.bramble.api.system; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @NotNullByDefault -public interface AndroidWakeLockFactory { +public interface AndroidWakeLockManager { AndroidWakeLock createWakeLock(); + + void runWakefully(Runnable r); } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothConnectionFactory.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothConnectionFactory.java index 61d543414..b14d5459b 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothConnectionFactory.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothConnectionFactory.java @@ -6,7 +6,7 @@ import org.briarproject.bramble.api.io.TimeoutMonitor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import java.io.IOException; @@ -15,15 +15,15 @@ class AndroidBluetoothConnectionFactory implements BluetoothConnectionFactory { private final BluetoothConnectionLimiter connectionLimiter; - private final AndroidWakeLockFactory wakeLockFactory; + private final AndroidWakeLockManager wakeLockManager; private final TimeoutMonitor timeoutMonitor; AndroidBluetoothConnectionFactory( BluetoothConnectionLimiter connectionLimiter, - AndroidWakeLockFactory wakeLockFactory, + AndroidWakeLockManager wakeLockManager, TimeoutMonitor timeoutMonitor) { this.connectionLimiter = connectionLimiter; - this.wakeLockFactory = wakeLockFactory; + this.wakeLockManager = wakeLockManager; this.timeoutMonitor = timeoutMonitor; } @@ -31,6 +31,6 @@ class AndroidBluetoothConnectionFactory public DuplexTransportConnection wrapSocket(DuplexPlugin plugin, BluetoothSocket s) throws IOException { return new AndroidBluetoothTransportConnection(plugin, - connectionLimiter, wakeLockFactory, timeoutMonitor, s); + connectionLimiter, wakeLockManager, timeoutMonitor, s); } } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java index 590117345..ecd6b95b6 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java @@ -13,7 +13,7 @@ import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory; import org.briarproject.bramble.api.system.AndroidExecutor; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.Clock; import java.security.SecureRandom; @@ -35,7 +35,7 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { private final Executor ioExecutor; private final AndroidExecutor androidExecutor; - private final AndroidWakeLockFactory wakeLockFactory; + private final AndroidWakeLockManager wakeLockManager; private final Context appContext; private final SecureRandom secureRandom; private final EventBus eventBus; @@ -45,7 +45,7 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { public AndroidBluetoothPluginFactory(Executor ioExecutor, AndroidExecutor androidExecutor, - AndroidWakeLockFactory wakeLockFactory, + AndroidWakeLockManager wakeLockManager, Context appContext, SecureRandom secureRandom, EventBus eventBus, @@ -54,7 +54,7 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { BackoffFactory backoffFactory) { this.ioExecutor = ioExecutor; this.androidExecutor = androidExecutor; - this.wakeLockFactory = wakeLockFactory; + this.wakeLockManager = wakeLockManager; this.appContext = appContext; this.secureRandom = secureRandom; this.eventBus = eventBus; @@ -79,7 +79,7 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { new BluetoothConnectionLimiterImpl(eventBus); BluetoothConnectionFactory connectionFactory = new AndroidBluetoothConnectionFactory(connectionLimiter, - wakeLockFactory, timeoutMonitor); + wakeLockManager, timeoutMonitor); Backoff backoff = backoffFactory.createBackoff(MIN_POLLING_INTERVAL, MAX_POLLING_INTERVAL, BACKOFF_BASE); AndroidBluetoothPlugin plugin = new AndroidBluetoothPlugin( diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java index ad0f4f038..3d500cbd7 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java @@ -7,7 +7,7 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.Plugin; import org.briarproject.bramble.api.plugin.duplex.AbstractDuplexTransportConnection; import org.briarproject.bramble.api.system.AndroidWakeLock; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import java.io.IOException; import java.io.InputStream; @@ -27,7 +27,7 @@ class AndroidBluetoothTransportConnection AndroidBluetoothTransportConnection(Plugin plugin, BluetoothConnectionLimiter connectionLimiter, - AndroidWakeLockFactory wakeLockFactory, + AndroidWakeLockManager wakeLockManager, TimeoutMonitor timeoutMonitor, BluetoothSocket socket) throws IOException { super(plugin); @@ -35,7 +35,7 @@ class AndroidBluetoothTransportConnection this.socket = socket; in = timeoutMonitor.createTimeoutInputStream( socket.getInputStream(), plugin.getMaxIdleTime() * 2); - wakeLock = wakeLockFactory.createWakeLock(); + wakeLock = wakeLockManager.createWakeLock(); wakeLock.acquire(); String address = socket.getRemoteDevice().getAddress(); if (isValidBluetoothAddress(address)) remote.put(PROP_ADDRESS, address); diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java index 391416019..a6bc002e5 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java @@ -12,7 +12,7 @@ import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.plugin.Backoff; import org.briarproject.bramble.api.plugin.PluginCallback; import org.briarproject.bramble.api.system.AndroidWakeLock; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.LocationUtils; import org.briarproject.bramble.api.system.ResourceProvider; @@ -40,7 +40,7 @@ class AndroidTorPlugin extends TorPlugin { ResourceProvider resourceProvider, CircumventionProvider circumventionProvider, BatteryManager batteryManager, - AndroidWakeLockFactory wakeLockFactory, + AndroidWakeLockManager wakeLockManager, Backoff backoff, TorRendezvousCrypto torRendezvousCrypto, PluginCallback callback, @@ -53,7 +53,7 @@ class AndroidTorPlugin extends TorPlugin { maxLatency, maxIdleTime, appContext.getDir("tor", MODE_PRIVATE)); this.appContext = appContext; - wakeLock = wakeLockFactory.createWakeLock(); + wakeLock = wakeLockManager.createWakeLock(); } @Override diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPluginFactory.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPluginFactory.java index 7f477a4d5..8f2b936ee 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPluginFactory.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPluginFactory.java @@ -13,7 +13,7 @@ import org.briarproject.bramble.api.plugin.TorConstants; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.LocationUtils; import org.briarproject.bramble.api.system.ResourceProvider; @@ -48,7 +48,7 @@ public class AndroidTorPluginFactory implements DuplexPluginFactory { private final ResourceProvider resourceProvider; private final CircumventionProvider circumventionProvider; private final BatteryManager batteryManager; - private final AndroidWakeLockFactory wakeLockFactory; + private final AndroidWakeLockManager wakeLockManager; private final Clock clock; public AndroidTorPluginFactory(Executor ioExecutor, @@ -61,7 +61,7 @@ public class AndroidTorPluginFactory implements DuplexPluginFactory { ResourceProvider resourceProvider, CircumventionProvider circumventionProvider, BatteryManager batteryManager, - AndroidWakeLockFactory wakeLockFactory, + AndroidWakeLockManager wakeLockManager, Clock clock) { this.ioExecutor = ioExecutor; this.appContext = appContext; @@ -73,7 +73,7 @@ public class AndroidTorPluginFactory implements DuplexPluginFactory { this.resourceProvider = resourceProvider; this.circumventionProvider = circumventionProvider; this.batteryManager = batteryManager; - this.wakeLockFactory = wakeLockFactory; + this.wakeLockManager = wakeLockManager; this.clock = clock; } @@ -120,7 +120,7 @@ public class AndroidTorPluginFactory implements DuplexPluginFactory { AndroidTorPlugin plugin = new AndroidTorPlugin(ioExecutor, appContext, networkManager, locationUtils, torSocketFactory, clock, resourceProvider, circumventionProvider, batteryManager, - wakeLockFactory, backoff, torRendezvousCrypto, callback, + wakeLockManager, backoff, torRendezvousCrypto, callback, architecture, MAX_LATENCY, MAX_IDLE_TIME); eventBus.addListener(plugin); return plugin; diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java index b18b99d26..27cb90b19 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java @@ -2,7 +2,7 @@ package org.briarproject.bramble.system; import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.system.AndroidExecutor; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.LocationUtils; import org.briarproject.bramble.api.system.ResourceProvider; import org.briarproject.bramble.api.system.SecureRandomProvider; @@ -51,8 +51,8 @@ public class AndroidSystemModule { @Provides @Singleton - AndroidWakeLockFactory provideWakeLockFactory( - AndroidWakeLockFactoryImpl wakeLockFactory) { - return wakeLockFactory; + AndroidWakeLockManager provideWakeLockManager( + AndroidWakeLockManagerImpl wakeLockManager) { + return wakeLockManager; } } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java index 3b8ade660..c20704ef9 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java @@ -12,7 +12,7 @@ import org.briarproject.bramble.api.lifecycle.Service; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AlarmListener; import org.briarproject.bramble.api.system.AndroidWakeLock; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.TaskScheduler; import java.util.ArrayList; @@ -53,7 +53,7 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { private static final long ALARM_MS = INTERVAL_FIFTEEN_MINUTES; private final Application app; - private final AndroidWakeLockFactory wakeLockFactory; + private final AndroidWakeLockManager wakeLockManager; private final ScheduledExecutorService scheduledExecutorService; private final AlarmManager alarmManager; @@ -62,10 +62,10 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { private final Queue tasks = new PriorityQueue<>(); AndroidTaskScheduler(Application app, - AndroidWakeLockFactory wakeLockFactory, + AndroidWakeLockManager wakeLockManager, ScheduledExecutorService scheduledExecutorService) { this.app = app; - this.wakeLockFactory = wakeLockFactory; + this.wakeLockManager = wakeLockManager; this.scheduledExecutorService = scheduledExecutorService; alarmManager = (AlarmManager) requireNonNull(app.getSystemService(ALARM_SERVICE)); @@ -73,7 +73,8 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { @Override public void startService() { - scheduledExecutorService.scheduleAtFixedRate(this::runDueTasks, + scheduledExecutorService.scheduleAtFixedRate( + () -> wakeLockManager.runWakefully(this::runDueTasks), TICK_MS, TICK_MS, MILLISECONDS); scheduleAlarm(); } @@ -112,21 +113,23 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { @Override public void onAlarm(Intent intent) { - int extraPid = intent.getIntExtra(EXTRA_PID, -1); - int currentPid = Process.myPid(); - if (extraPid == currentPid) { - LOG.info("Alarm"); - rescheduleAlarm(); - runDueTasks(); - } else { - LOG.info("Ignoring alarm with PID " + extraPid - + ", current PID is " + currentPid); - } + wakeLockManager.runWakefully(() -> { + int extraPid = intent.getIntExtra(EXTRA_PID, -1); + int currentPid = Process.myPid(); + if (extraPid == currentPid) { + LOG.info("Alarm"); + rescheduleAlarm(); + runDueTasks(); + } else { + LOG.info("Ignoring alarm with PID " + extraPid + + ", current PID is " + currentPid); + } + }); } private Runnable createWakefulTask(Runnable task, Executor executor) { // Hold a wake lock from before we submit the task until after it runs - AndroidWakeLock wakeLock = wakeLockFactory.createWakeLock(); + AndroidWakeLock wakeLock = wakeLockManager.createWakeLock(); return () -> { wakeLock.acquire(); executor.execute(() -> { diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java index fd47e7523..1feeb5d0b 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java @@ -4,7 +4,7 @@ import android.app.Application; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.system.AlarmListener; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.TaskScheduler; import java.util.concurrent.RejectedExecutionHandler; @@ -38,10 +38,10 @@ public class AndroidTaskSchedulerModule { @Singleton AndroidTaskScheduler provideAndroidTaskScheduler( LifecycleManager lifecycleManager, Application app, - AndroidWakeLockFactory wakeLockFactory) { + AndroidWakeLockManager wakeLockManager) { lifecycleManager.registerForShutdown(scheduledExecutorService); AndroidTaskScheduler scheduler = new AndroidTaskScheduler(app, - wakeLockFactory, scheduledExecutorService); + wakeLockManager, scheduledExecutorService); lifecycleManager.registerService(scheduler); return scheduler; } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockFactoryImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java similarity index 85% rename from bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockFactoryImpl.java rename to bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java index 01ee5f563..988c8cd54 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockFactoryImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java @@ -9,7 +9,7 @@ import android.os.PowerManager; import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AndroidWakeLock; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.TaskScheduler; import java.util.concurrent.Executor; @@ -25,7 +25,7 @@ import static org.briarproject.bramble.api.nullsafety.NullSafety.requireNonNull; @Immutable @NotNullByDefault -class AndroidWakeLockFactoryImpl implements AndroidWakeLockFactory { +class AndroidWakeLockManagerImpl implements AndroidWakeLockManager { /** * How often to replace the wake lock. @@ -41,7 +41,7 @@ class AndroidWakeLockFactoryImpl implements AndroidWakeLockFactory { private final SharedWakeLock sharedWakeLock; @Inject - AndroidWakeLockFactoryImpl(TaskScheduler scheduler, + AndroidWakeLockManagerImpl(TaskScheduler scheduler, @EventExecutor Executor eventExecutor, Application app) { PowerManager powerManager = (PowerManager) @@ -57,6 +57,17 @@ class AndroidWakeLockFactoryImpl implements AndroidWakeLockFactory { return new AndroidWakeLockImpl(sharedWakeLock); } + @Override + public void runWakefully(Runnable r) { + AndroidWakeLock wakeLock = createWakeLock(); + wakeLock.acquire(); + try { + r.run(); + } finally { + wakeLock.release(); + } + } + private String getWakeLockTag(Context ctx) { PackageManager pm = ctx.getPackageManager(); for (PackageInfo info : pm.getInstalledPackages(0)) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java index b8ba76a9e..4e094b782 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java @@ -28,7 +28,7 @@ import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory; import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory; import org.briarproject.bramble.api.reporting.DevConfig; import org.briarproject.bramble.api.system.AndroidExecutor; -import org.briarproject.bramble.api.system.AndroidWakeLockFactory; +import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.LocationUtils; import org.briarproject.bramble.api.system.ResourceProvider; @@ -135,17 +135,17 @@ public class AppModule { ResourceProvider resourceProvider, CircumventionProvider circumventionProvider, BatteryManager batteryManager, - AndroidWakeLockFactory wakeLockFactory, + AndroidWakeLockManager wakeLockManager, Clock clock, TimeoutMonitor timeoutMonitor) { Context appContext = app.getApplicationContext(); DuplexPluginFactory bluetooth = new AndroidBluetoothPluginFactory( - ioExecutor, androidExecutor, wakeLockFactory, appContext, + ioExecutor, androidExecutor, wakeLockManager, appContext, random, eventBus, clock, timeoutMonitor, backoffFactory); DuplexPluginFactory tor = new AndroidTorPluginFactory(ioExecutor, appContext, networkManager, locationUtils, eventBus, torSocketFactory, backoffFactory, resourceProvider, - circumventionProvider, batteryManager, wakeLockFactory, clock); + circumventionProvider, batteryManager, wakeLockManager, clock); DuplexPluginFactory lan = new AndroidLanTcpPluginFactory(ioExecutor, eventBus, backoffFactory, appContext); Collection duplex = asList(bluetooth, tor, lan); From b2840c1b00dfcc04a5557e1d4fc776ab8e156957 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 6 Aug 2020 15:25:37 +0100 Subject: [PATCH 04/11] Add method for executing a task on an executor wakefully. --- .../api/system/AndroidWakeLockManager.java | 4 ++++ .../bramble/system/AndroidTaskScheduler.java | 19 ++----------------- .../system/AndroidWakeLockManagerImpl.java | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java index ba6c3ebc3..d1e9beff4 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java @@ -2,10 +2,14 @@ package org.briarproject.bramble.api.system; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import java.util.concurrent.Executor; + @NotNullByDefault public interface AndroidWakeLockManager { AndroidWakeLock createWakeLock(); void runWakefully(Runnable r); + + void executeWakefully(Runnable r, Executor executor); } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java index c20704ef9..773fbc09e 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java @@ -11,7 +11,6 @@ import android.os.SystemClock; import org.briarproject.bramble.api.lifecycle.Service; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AlarmListener; -import org.briarproject.bramble.api.system.AndroidWakeLock; import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.TaskScheduler; @@ -89,7 +88,8 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { TimeUnit unit) { long now = SystemClock.elapsedRealtime(); long dueMillis = now + MILLISECONDS.convert(delay, unit); - Runnable wakeful = createWakefulTask(task, executor); + Runnable wakeful = () -> + wakeLockManager.executeWakefully(task, executor); ScheduledTask s = new ScheduledTask(wakeful, dueMillis); if (dueMillis <= now) { scheduledExecutorService.execute(s); @@ -127,21 +127,6 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { }); } - private Runnable createWakefulTask(Runnable task, Executor executor) { - // Hold a wake lock from before we submit the task until after it runs - AndroidWakeLock wakeLock = wakeLockManager.createWakeLock(); - return () -> { - wakeLock.acquire(); - executor.execute(() -> { - try { - task.run(); - } finally { - wakeLock.release(); - } - }); - }; - } - private void runDueTasks() { long now = SystemClock.elapsedRealtime(); List due = new ArrayList<>(); diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java index 988c8cd54..187d72ab3 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java @@ -68,6 +68,24 @@ class AndroidWakeLockManagerImpl implements AndroidWakeLockManager { } } + @Override + public void executeWakefully(Runnable r, Executor executor) { + AndroidWakeLock wakeLock = createWakeLock(); + wakeLock.acquire(); + try { + executor.execute(() -> { + try { + r.run(); + } finally { + wakeLock.release(); + } + }); + } catch (Exception e) { + wakeLock.release(); + throw e; + } + } + private String getWakeLockTag(Context ctx) { PackageManager pm = ctx.getPackageManager(); for (PackageInfo info : pm.getInstalledPackages(0)) { From e6c3f82fe2e5e28ff5b0374a6087d7ca5cca6c5a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 6 Aug 2020 15:38:16 +0100 Subject: [PATCH 05/11] Fix test expectations. --- .../bramble/transport/TransportKeyManagerImplTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java index 084e15463..f5b93c82a 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java @@ -424,8 +424,6 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { with(dbExecutor), with(timePeriodLength), with(MILLISECONDS)); will(new RunAction()); - oneOf(dbExecutor).execute(with(any(Runnable.class))); - will(new RunAction()); // Start a transaction for updating keys oneOf(db).transaction(with(false), withDbRunnable(txn1)); // Get the current time (the start of time period 1001) From af1a91c819c1e82ac8646c9acd834aa9341e3833 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 6 Aug 2020 16:10:50 +0100 Subject: [PATCH 06/11] Fix circular dependency between scheduler and wake lock manager. --- .../bramble/system/AndroidSystemModule.java | 21 +++++++++++++++++++ .../system/AndroidTaskSchedulerModule.java | 15 ++----------- .../system/AndroidWakeLockManagerImpl.java | 14 ++++++------- .../bramble/system/RenewableWakeLock.java | 18 +++++++--------- .../bramble/system/TaskSchedulerImpl.java | 11 +++++----- 5 files changed, 42 insertions(+), 37 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java index 27cb90b19..714d1ab83 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidSystemModule.java @@ -1,6 +1,7 @@ package org.briarproject.bramble.system; import org.briarproject.bramble.api.event.EventExecutor; +import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.system.AndroidExecutor; import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.LocationUtils; @@ -8,6 +9,9 @@ import org.briarproject.bramble.api.system.ResourceProvider; import org.briarproject.bramble.api.system.SecureRandomProvider; import java.util.concurrent.Executor; +import java.util.concurrent.RejectedExecutionHandler; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import javax.inject.Singleton; @@ -17,6 +21,23 @@ import dagger.Provides; @Module public class AndroidSystemModule { + private final ScheduledExecutorService scheduledExecutorService; + + public AndroidSystemModule() { + // Discard tasks that are submitted during shutdown + RejectedExecutionHandler policy = + new ScheduledThreadPoolExecutor.DiscardPolicy(); + scheduledExecutorService = new ScheduledThreadPoolExecutor(1, policy); + } + + @Provides + @Singleton + ScheduledExecutorService provideScheduledExecutorService( + LifecycleManager lifecycleManager) { + lifecycleManager.registerForShutdown(scheduledExecutorService); + return scheduledExecutorService; + } + @Provides @Singleton SecureRandomProvider provideSecureRandomProvider( diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java index 1feeb5d0b..2dcbb551c 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskSchedulerModule.java @@ -7,9 +7,7 @@ import org.briarproject.bramble.api.system.AlarmListener; import org.briarproject.bramble.api.system.AndroidWakeLockManager; import org.briarproject.bramble.api.system.TaskScheduler; -import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledThreadPoolExecutor; import javax.inject.Inject; import javax.inject.Singleton; @@ -25,21 +23,12 @@ public class AndroidTaskSchedulerModule { AndroidTaskScheduler scheduler; } - private final ScheduledExecutorService scheduledExecutorService; - - public AndroidTaskSchedulerModule() { - // Discard tasks that are submitted during shutdown - RejectedExecutionHandler policy = - new ScheduledThreadPoolExecutor.DiscardPolicy(); - scheduledExecutorService = new ScheduledThreadPoolExecutor(1, policy); - } - @Provides @Singleton AndroidTaskScheduler provideAndroidTaskScheduler( LifecycleManager lifecycleManager, Application app, - AndroidWakeLockManager wakeLockManager) { - lifecycleManager.registerForShutdown(scheduledExecutorService); + AndroidWakeLockManager wakeLockManager, + ScheduledExecutorService scheduledExecutorService) { AndroidTaskScheduler scheduler = new AndroidTaskScheduler(app, wakeLockManager, scheduledExecutorService); lifecycleManager.registerService(scheduler); diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java index 187d72ab3..1500d148b 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java @@ -6,13 +6,12 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.os.PowerManager; -import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AndroidWakeLock; import org.briarproject.bramble.api.system.AndroidWakeLockManager; -import org.briarproject.bramble.api.system.TaskScheduler; import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; @@ -41,15 +40,14 @@ class AndroidWakeLockManagerImpl implements AndroidWakeLockManager { private final SharedWakeLock sharedWakeLock; @Inject - AndroidWakeLockManagerImpl(TaskScheduler scheduler, - @EventExecutor Executor eventExecutor, - Application app) { + AndroidWakeLockManagerImpl(Application app, + ScheduledExecutorService scheduledExecutorService) { PowerManager powerManager = (PowerManager) requireNonNull(app.getSystemService(POWER_SERVICE)); String tag = getWakeLockTag(app); - sharedWakeLock = new RenewableWakeLock(powerManager, scheduler, - eventExecutor, PARTIAL_WAKE_LOCK, tag, LOCK_DURATION_MS, - SAFETY_MARGIN_MS); + sharedWakeLock = new RenewableWakeLock(powerManager, + scheduledExecutorService, PARTIAL_WAKE_LOCK, tag, + LOCK_DURATION_MS, SAFETY_MARGIN_MS); } @Override diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java b/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java index a2852c17e..c04212293 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java @@ -4,10 +4,9 @@ import android.os.PowerManager; import android.os.PowerManager.WakeLock; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import org.briarproject.bramble.api.system.TaskScheduler; -import java.util.concurrent.Executor; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -28,8 +27,7 @@ class RenewableWakeLock implements SharedWakeLock { getLogger(RenewableWakeLock.class.getName()); private final PowerManager powerManager; - private final TaskScheduler scheduler; - private final Executor eventExecutor; + private final ScheduledExecutorService scheduledExecutorService; private final int levelAndFlags; private final String tag; private final long durationMs, safetyMarginMs; @@ -47,15 +45,13 @@ class RenewableWakeLock implements SharedWakeLock { private long acquired = 0; RenewableWakeLock(PowerManager powerManager, - TaskScheduler scheduler, - Executor eventExecutor, + ScheduledExecutorService scheduledExecutorService, int levelAndFlags, String tag, long durationMs, long safetyMarginMs) { this.powerManager = powerManager; - this.scheduler = scheduler; - this.eventExecutor = eventExecutor; + this.scheduledExecutorService = scheduledExecutorService; this.levelAndFlags = levelAndFlags; this.tag = tag; this.durationMs = durationMs; @@ -76,7 +72,7 @@ class RenewableWakeLock implements SharedWakeLock { // power management apps wakeLock.setReferenceCounted(false); wakeLock.acquire(durationMs + safetyMarginMs); - future = scheduler.schedule(this::renew, eventExecutor, + future = scheduledExecutorService.schedule(this::renew, durationMs, MILLISECONDS); acquired = android.os.SystemClock.elapsedRealtime(); } @@ -100,8 +96,8 @@ class RenewableWakeLock implements SharedWakeLock { wakeLock.setReferenceCounted(false); wakeLock.acquire(durationMs + safetyMarginMs); oldWakeLock.release(); - future = scheduler.schedule(this::renew, eventExecutor, - durationMs, MILLISECONDS); + future = scheduledExecutorService.schedule(this::renew, durationMs, + MILLISECONDS); acquired = now; } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java index 1627cad0f..629f2ecd2 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/system/TaskSchedulerImpl.java @@ -17,23 +17,24 @@ import javax.annotation.concurrent.ThreadSafe; @NotNullByDefault class TaskSchedulerImpl implements TaskScheduler { - private final ScheduledExecutorService delegate; + private final ScheduledExecutorService scheduledExecutorService; - TaskSchedulerImpl(ScheduledExecutorService delegate) { - this.delegate = delegate; + TaskSchedulerImpl(ScheduledExecutorService scheduledExecutorService) { + this.scheduledExecutorService = scheduledExecutorService; } @Override public Future schedule(Runnable task, Executor executor, long delay, TimeUnit unit) { Runnable execute = () -> executor.execute(task); - return delegate.schedule(execute, delay, unit); + return scheduledExecutorService.schedule(execute, delay, unit); } @Override public Future scheduleWithFixedDelay(Runnable task, Executor executor, long delay, long interval, TimeUnit unit) { Runnable execute = () -> executor.execute(task); - return delegate.scheduleWithFixedDelay(execute, delay, interval, unit); + return scheduledExecutorService.scheduleWithFixedDelay(execute, delay, + interval, unit); } } From 1bab15baaface2351a883b53c623c116deb8da4d Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 7 Aug 2020 11:57:35 +0100 Subject: [PATCH 07/11] Add fine logging for wake locks. --- .../bramble/system/AndroidWakeLockImpl.java | 15 ++++++++++++++- .../bramble/system/RenewableWakeLock.java | 8 ++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java index d3d9a7c4b..0efd352a0 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java @@ -3,9 +3,13 @@ package org.briarproject.bramble.system; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AndroidWakeLock; +import java.util.logging.Logger; + import javax.annotation.concurrent.GuardedBy; import javax.annotation.concurrent.ThreadSafe; +import static java.util.logging.Logger.getLogger; + /** * A wrapper around a {@link SharedWakeLock} that provides the more convenient * semantics of {@link AndroidWakeLock} (i.e. calls to acquire() and release() @@ -15,6 +19,9 @@ import javax.annotation.concurrent.ThreadSafe; @NotNullByDefault class AndroidWakeLockImpl implements AndroidWakeLock { + private static final Logger LOG = + getLogger(AndroidWakeLockImpl.class.getName()); + private final SharedWakeLock sharedWakeLock; private final Object lock = new Object(); @GuardedBy("lock") @@ -27,7 +34,10 @@ class AndroidWakeLockImpl implements AndroidWakeLock { @Override public void acquire() { synchronized (lock) { - if (!held) { + if (held) { + LOG.fine("Already acquired"); + } else { + LOG.fine("Acquiring shared wake lock"); held = true; sharedWakeLock.acquire(); } @@ -38,8 +48,11 @@ class AndroidWakeLockImpl implements AndroidWakeLock { public void release() { synchronized (lock) { if (held) { + LOG.fine("Releasing shared wake lock"); held = false; sharedWakeLock.release(); + } else { + LOG.fine("Already released"); } } } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java b/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java index c04212293..122180fd6 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/RenewableWakeLock.java @@ -14,6 +14,7 @@ import javax.annotation.concurrent.GuardedBy; import javax.annotation.concurrent.ThreadSafe; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.logging.Level.FINE; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; @@ -75,6 +76,8 @@ class RenewableWakeLock implements SharedWakeLock { future = scheduledExecutorService.schedule(this::renew, durationMs, MILLISECONDS); acquired = android.os.SystemClock.elapsedRealtime(); + } else if (LOG.isLoggable(FINE)) { + LOG.fine("Wake lock " + tag + " has " + refCount + " holders"); } } } @@ -86,6 +89,9 @@ class RenewableWakeLock implements SharedWakeLock { LOG.info("Already released"); return; } + if (LOG.isLoggable(FINE)) { + LOG.fine("Wake lock " + tag + " has " + refCount + " holders"); + } long now = android.os.SystemClock.elapsedRealtime(); long expiry = acquired + durationMs + safetyMarginMs; if (now > expiry && LOG.isLoggable(WARNING)) { @@ -115,6 +121,8 @@ class RenewableWakeLock implements SharedWakeLock { requireNonNull(wakeLock).release(); wakeLock = null; acquired = 0; + } else if (LOG.isLoggable(FINE)) { + LOG.fine("Wake lock " + tag + " has " + refCount + " holders"); } } } From 7e0d21de387aee304dcd85cd85d13ff18ce429b5 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 7 Aug 2020 12:06:03 +0100 Subject: [PATCH 08/11] Add tags for wake lock holders. --- .../api/system/AndroidWakeLockManager.java | 6 ++--- .../AndroidBluetoothTransportConnection.java | 2 +- .../bramble/plugin/tor/AndroidTorPlugin.java | 2 +- .../bramble/system/AndroidTaskScheduler.java | 8 +++---- .../bramble/system/AndroidWakeLockImpl.java | 22 ++++++++++++++----- .../system/AndroidWakeLockManagerImpl.java | 12 +++++----- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java index d1e9beff4..66f7c4b44 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java @@ -7,9 +7,9 @@ import java.util.concurrent.Executor; @NotNullByDefault public interface AndroidWakeLockManager { - AndroidWakeLock createWakeLock(); + AndroidWakeLock createWakeLock(String tag); - void runWakefully(Runnable r); + void runWakefully(Runnable r, String tag); - void executeWakefully(Runnable r, Executor executor); + void executeWakefully(Runnable r, Executor executor, String tag); } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java index 3d500cbd7..b6a695116 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java @@ -35,7 +35,7 @@ class AndroidBluetoothTransportConnection this.socket = socket; in = timeoutMonitor.createTimeoutInputStream( socket.getInputStream(), plugin.getMaxIdleTime() * 2); - wakeLock = wakeLockManager.createWakeLock(); + wakeLock = wakeLockManager.createWakeLock("BluetoothConnection"); wakeLock.acquire(); String address = socket.getRemoteDevice().getAddress(); if (isValidBluetoothAddress(address)) remote.put(PROP_ADDRESS, address); diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java index a6bc002e5..79e35151c 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/AndroidTorPlugin.java @@ -53,7 +53,7 @@ class AndroidTorPlugin extends TorPlugin { maxLatency, maxIdleTime, appContext.getDir("tor", MODE_PRIVATE)); this.appContext = appContext; - wakeLock = wakeLockManager.createWakeLock(); + wakeLock = wakeLockManager.createWakeLock("TorPlugin"); } @Override diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java index 773fbc09e..3af47ebee 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidTaskScheduler.java @@ -73,8 +73,8 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { @Override public void startService() { scheduledExecutorService.scheduleAtFixedRate( - () -> wakeLockManager.runWakefully(this::runDueTasks), - TICK_MS, TICK_MS, MILLISECONDS); + () -> wakeLockManager.runWakefully(this::runDueTasks, + "TaskTicker"), TICK_MS, TICK_MS, MILLISECONDS); scheduleAlarm(); } @@ -89,7 +89,7 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { long now = SystemClock.elapsedRealtime(); long dueMillis = now + MILLISECONDS.convert(delay, unit); Runnable wakeful = () -> - wakeLockManager.executeWakefully(task, executor); + wakeLockManager.executeWakefully(task, executor, "TaskHandoff"); ScheduledTask s = new ScheduledTask(wakeful, dueMillis); if (dueMillis <= now) { scheduledExecutorService.execute(s); @@ -124,7 +124,7 @@ class AndroidTaskScheduler implements TaskScheduler, Service, AlarmListener { LOG.info("Ignoring alarm with PID " + extraPid + ", current PID is " + currentPid); } - }); + }, "TaskAlarm"); } private void runDueTasks() { diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java index 0efd352a0..ecd3136d9 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java @@ -8,6 +8,7 @@ import java.util.logging.Logger; import javax.annotation.concurrent.GuardedBy; import javax.annotation.concurrent.ThreadSafe; +import static java.util.logging.Level.FINE; import static java.util.logging.Logger.getLogger; /** @@ -23,21 +24,28 @@ class AndroidWakeLockImpl implements AndroidWakeLock { getLogger(AndroidWakeLockImpl.class.getName()); private final SharedWakeLock sharedWakeLock; + private final String tag; + private final Object lock = new Object(); @GuardedBy("lock") private boolean held = false; - AndroidWakeLockImpl(SharedWakeLock sharedWakeLock) { + AndroidWakeLockImpl(SharedWakeLock sharedWakeLock, String tag) { this.sharedWakeLock = sharedWakeLock; + this.tag = tag; } @Override public void acquire() { synchronized (lock) { if (held) { - LOG.fine("Already acquired"); + if (LOG.isLoggable(FINE)) { + LOG.fine(tag + " already acquired"); + } } else { - LOG.fine("Acquiring shared wake lock"); + if (LOG.isLoggable(FINE)) { + LOG.fine(tag + " acquiring shared wake lock"); + } held = true; sharedWakeLock.acquire(); } @@ -48,11 +56,15 @@ class AndroidWakeLockImpl implements AndroidWakeLock { public void release() { synchronized (lock) { if (held) { - LOG.fine("Releasing shared wake lock"); + if (LOG.isLoggable(FINE)) { + LOG.fine(tag + " releasing shared wake lock"); + } held = false; sharedWakeLock.release(); } else { - LOG.fine("Already released"); + if (LOG.isLoggable(FINE)) { + LOG.fine(tag + " already released"); + } } } } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java index 1500d148b..323bb9bc9 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java @@ -51,13 +51,13 @@ class AndroidWakeLockManagerImpl implements AndroidWakeLockManager { } @Override - public AndroidWakeLock createWakeLock() { - return new AndroidWakeLockImpl(sharedWakeLock); + public AndroidWakeLock createWakeLock(String tag) { + return new AndroidWakeLockImpl(sharedWakeLock, tag); } @Override - public void runWakefully(Runnable r) { - AndroidWakeLock wakeLock = createWakeLock(); + public void runWakefully(Runnable r, String tag) { + AndroidWakeLock wakeLock = createWakeLock(tag); wakeLock.acquire(); try { r.run(); @@ -67,8 +67,8 @@ class AndroidWakeLockManagerImpl implements AndroidWakeLockManager { } @Override - public void executeWakefully(Runnable r, Executor executor) { - AndroidWakeLock wakeLock = createWakeLock(); + public void executeWakefully(Runnable r, Executor executor, String tag) { + AndroidWakeLock wakeLock = createWakeLock(tag); wakeLock.acquire(); try { executor.execute(() -> { From d8be3401209d647b7effe4c76dcc6cf80a7cf4dc Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 7 Aug 2020 14:38:07 +0100 Subject: [PATCH 09/11] Use a unique log tag for each wake lock instance. --- .../org/briarproject/bramble/system/AndroidWakeLockImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java index ecd3136d9..625d43760 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockImpl.java @@ -3,6 +3,7 @@ package org.briarproject.bramble.system; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AndroidWakeLock; +import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; import javax.annotation.concurrent.GuardedBy; @@ -23,6 +24,8 @@ class AndroidWakeLockImpl implements AndroidWakeLock { private static final Logger LOG = getLogger(AndroidWakeLockImpl.class.getName()); + private static final AtomicInteger INSTANCE_ID = new AtomicInteger(0); + private final SharedWakeLock sharedWakeLock; private final String tag; @@ -32,7 +35,7 @@ class AndroidWakeLockImpl implements AndroidWakeLock { AndroidWakeLockImpl(SharedWakeLock sharedWakeLock, String tag) { this.sharedWakeLock = sharedWakeLock; - this.tag = tag; + this.tag = tag + "_" + INSTANCE_ID.getAndIncrement(); } @Override From 6e8e955dc287f5dccb2087ebe70da0e5dcacc866 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 7 Aug 2020 16:16:31 +0100 Subject: [PATCH 10/11] Update javadocs. --- .../bramble/api/system/AndroidWakeLockManager.java | 12 ++++++++++++ .../bramble/api/system/TaskScheduler.java | 3 --- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java index 66f7c4b44..0ebf256a2 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/api/system/AndroidWakeLockManager.java @@ -7,9 +7,21 @@ import java.util.concurrent.Executor; @NotNullByDefault public interface AndroidWakeLockManager { + /** + * Creates a wake lock with the given tag. The tag is only used for + * logging; the underlying OS wake lock will use its own tag. + */ AndroidWakeLock createWakeLock(String tag); + /** + * Runs the given task while holding a wake lock. + */ void runWakefully(Runnable r, String tag); + /** + * Submits the given task to the given executor while holding a wake lock. + * The lock is released when the task completes, or if an exception is + * thrown while submitting or running the task. + */ void executeWakefully(Runnable r, Executor executor, String tag); } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java b/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java index 96c977dd4..9d72c4cfb 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/system/TaskScheduler.java @@ -8,9 +8,6 @@ import java.util.concurrent.TimeUnit; /** * A service that can be used to schedule the execution of tasks. - *

- * The service should only be used for running tasks on other executors - * at scheduled times. No significant work should be run by the service itself. */ @NotNullByDefault public interface TaskScheduler { From eac93f43d39495cd0cb799890457c52cf8445897 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 10 Aug 2020 11:36:05 +0100 Subject: [PATCH 11/11] Add comments for wake lock handling. --- .../bramble/system/AndroidWakeLockManagerImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java index 323bb9bc9..4ed400eee 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/system/AndroidWakeLockManagerImpl.java @@ -75,10 +75,14 @@ class AndroidWakeLockManagerImpl implements AndroidWakeLockManager { try { r.run(); } finally { + // Release the wake lock if the task throws an exception wakeLock.release(); } }); } catch (Exception e) { + // Release the wake lock if the executor throws an exception when + // we submit the task (in which case the release() call above won't + // happen) wakeLock.release(); throw e; }