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 02a6d38e2..11694ba92 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 @@ -14,6 +14,7 @@ import android.net.NetworkInfo; import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.lifecycle.Service; @@ -23,7 +24,6 @@ import org.briarproject.bramble.api.network.event.NetworkStatusEvent; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.system.TaskScheduler; -import org.briarproject.bramble.api.system.TaskScheduler.Cancellable; import java.net.Inet4Address; import java.net.InetAddress; 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 fd0a89c13..c426f7d6a 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 @@ -8,6 +8,7 @@ import android.content.Intent; import android.os.Process; import android.os.SystemClock; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.lifecycle.Service; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.AlarmListener; diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/Cancellable.java b/bramble-api/src/main/java/org/briarproject/bramble/api/Cancellable.java new file mode 100644 index 000000000..286539b0f --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/Cancellable.java @@ -0,0 +1,6 @@ +package org.briarproject.bramble.api; + +public interface Cancellable { + + void cancel(); +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/Supplier.java b/bramble-api/src/main/java/org/briarproject/bramble/api/Supplier.java new file mode 100644 index 000000000..7a83bddb6 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/Supplier.java @@ -0,0 +1,9 @@ +package org.briarproject.bramble.api; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +@NotNullByDefault +public interface Supplier { + + T get(); +} 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 de1b85e8a..8c3b599b2 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 @@ -1,5 +1,6 @@ package org.briarproject.bramble.api.system; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.util.concurrent.Executor; @@ -16,6 +17,8 @@ public interface TaskScheduler { *

* If the platform supports wake locks, a wake lock will be held while * submitting and running the task. + * + * @return A {@link Cancellable} for cancelling the task. */ Cancellable schedule(Runnable task, Executor executor, long delay, TimeUnit unit); @@ -27,17 +30,11 @@ public interface TaskScheduler { *

* If the platform supports wake locks, a wake lock will be held while * submitting and running the task. + * + * @return A {@link Cancellable} for cancelling all future executions of + * the task. */ Cancellable scheduleWithFixedDelay(Runnable task, Executor executor, long delay, long interval, TimeUnit unit); - interface Cancellable { - - /** - * Cancels the task if it has not already started running. If the task - * is {@link #scheduleWithFixedDelay(Runnable, Executor, long, long, TimeUnit) periodic}, - * all future executions of the task are cancelled. - */ - void cancel(); - } } 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 d080cabeb..144fe1056 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 @@ -1,10 +1,10 @@ package org.briarproject.bramble.io; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.io.TimeoutMonitor; import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler; -import org.briarproject.bramble.api.system.TaskScheduler.Cancellable; import org.briarproject.bramble.api.system.Wakeful; import java.io.IOException; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCaller.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCaller.java new file mode 100644 index 000000000..7280eb0a1 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCaller.java @@ -0,0 +1,37 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.Cancellable; +import org.briarproject.bramble.api.Supplier; +import org.briarproject.bramble.api.lifecycle.IoExecutor; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; + +import static java.util.concurrent.TimeUnit.DAYS; +import static java.util.concurrent.TimeUnit.MINUTES; + +@NotNullByDefault +interface MailboxApiCaller { + + /** + * The minimum interval between retries in milliseconds. + */ + long MIN_RETRY_INTERVAL_MS = MINUTES.toMillis(1); + + /** + * The maximum interval between retries in milliseconds. + */ + long MAX_RETRY_INTERVAL_MS = DAYS.toMillis(1); + + /** + * Asynchronously calls the given supplier, automatically retrying at + * increasing intervals until the supplier returns false. The returned + * {@link Cancellable} can be used to cancel any future retries. + * + * @param supplier A wrapper for an API call. The supplier's + * {@link Supplier#get() get()} method will be called on the + * {@link IoExecutor}. It should return true if the API call needs to be + * retried, or false if the API call succeeded or + * {@link TolerableFailureException failed tolerably}. + */ + Cancellable retryWithBackoff(Supplier supplier); +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCallerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCallerImpl.java new file mode 100644 index 000000000..9b476eb98 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApiCallerImpl.java @@ -0,0 +1,99 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.Cancellable; +import org.briarproject.bramble.api.Supplier; +import org.briarproject.bramble.api.lifecycle.IoExecutor; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.system.TaskScheduler; + +import java.util.concurrent.Executor; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; +import javax.annotation.concurrent.Immutable; +import javax.inject.Inject; + +import static java.lang.Math.min; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + +@Immutable +@NotNullByDefault +class MailboxApiCallerImpl implements MailboxApiCaller { + + private final TaskScheduler taskScheduler; + private final Executor ioExecutor; + + @Inject + MailboxApiCallerImpl(TaskScheduler taskScheduler, + @IoExecutor Executor ioExecutor) { + this.taskScheduler = taskScheduler; + this.ioExecutor = ioExecutor; + } + + @Override + public Cancellable retryWithBackoff(Supplier supplier) { + Task task = new Task(supplier); + task.start(); + return task; + } + + private class Task implements Cancellable { + + private final Supplier supplier; + private final Object lock = new Object(); + + @GuardedBy("lock") + @Nullable + private Cancellable scheduledTask = null; + + @GuardedBy("lock") + private boolean cancelled = false; + + @GuardedBy("lock") + private long retryIntervalMs = MIN_RETRY_INTERVAL_MS; + + private Task(Supplier supplier) { + this.supplier = supplier; + } + + private void start() { + synchronized (lock) { + if (cancelled) throw new AssertionError(); + ioExecutor.execute(this::callApi); + } + } + + @IoExecutor + private void callApi() { + synchronized (lock) { + if (cancelled) return; + } + // The supplier returns true if we should retry + if (supplier.get()) { + synchronized (lock) { + if (cancelled) return; + scheduledTask = taskScheduler.schedule(this::callApi, + ioExecutor, retryIntervalMs, MILLISECONDS); + // Increase the retry interval each time we retry + retryIntervalMs = + min(MAX_RETRY_INTERVAL_MS, retryIntervalMs * 2); + } + } else { + synchronized (lock) { + scheduledTask = null; + } + } + } + + @Override + public void cancel() { + Cancellable scheduledTask; + synchronized (lock) { + cancelled = true; + scheduledTask = this.scheduledTask; + this.scheduledTask = null; + } + if (scheduledTask != null) scheduledTask.cancel(); + } + } +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/SimpleApiCall.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/SimpleApiCall.java new file mode 100644 index 000000000..14c508b33 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/SimpleApiCall.java @@ -0,0 +1,40 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.Supplier; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.mailbox.MailboxApi.ApiException; +import org.briarproject.bramble.mailbox.MailboxApi.TolerableFailureException; + +import java.io.IOException; +import java.util.logging.Logger; + +import static java.util.logging.Level.INFO; +import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.util.LogUtils.logException; + +/** + * Convenience class for making simple API calls that don't return values. + */ +@NotNullByDefault +public abstract class SimpleApiCall implements Supplier { + + private static final Logger LOG = getLogger(SimpleApiCall.class.getName()); + + abstract void callApi() + throws IOException, ApiException, TolerableFailureException; + + @Override + public Boolean get() { + try { + callApi(); + return false; // Succeeded, don't retry + } catch (IOException | ApiException e) { + logException(LOG, WARNING, e); + return true; // Failed, retry with backoff + } catch (TolerableFailureException e) { + logException(LOG, INFO, e); + return false; // Failed tolerably, don't retry + } + } +} 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 4148d15f2..873a19e7a 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 @@ -1,5 +1,6 @@ package org.briarproject.bramble.plugin; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.Pair; import org.briarproject.bramble.api.connection.ConnectionManager; import org.briarproject.bramble.api.connection.ConnectionRegistry; @@ -27,7 +28,6 @@ import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler; -import org.briarproject.bramble.api.system.TaskScheduler.Cancellable; import org.briarproject.bramble.api.system.Wakeful; import org.briarproject.bramble.api.system.WakefulIoExecutor; 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 769aae93f..54cc2ef08 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 @@ -1,6 +1,7 @@ package org.briarproject.bramble.rendezvous; import org.briarproject.bramble.PoliteExecutor; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.Pair; import org.briarproject.bramble.api.connection.ConnectionManager; import org.briarproject.bramble.api.contact.PendingContact; @@ -42,7 +43,6 @@ import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionOpenedE import org.briarproject.bramble.api.rendezvous.event.RendezvousPollEvent; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler; -import org.briarproject.bramble.api.system.TaskScheduler.Cancellable; import org.briarproject.bramble.api.system.Wakeful; import java.security.GeneralSecurityException; 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 ef797a28c..363821537 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 @@ -1,5 +1,6 @@ package org.briarproject.bramble.system; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.TaskScheduler; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiCallerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiCallerImplTest.java new file mode 100644 index 000000000..10bb91745 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/MailboxApiCallerImplTest.java @@ -0,0 +1,141 @@ +package org.briarproject.bramble.mailbox; + +import org.briarproject.bramble.api.Cancellable; +import org.briarproject.bramble.api.Supplier; +import org.briarproject.bramble.api.system.TaskScheduler; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.briarproject.bramble.test.CaptureArgumentAction; +import org.jmock.Expectations; +import org.jmock.lib.action.DoAllAction; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.briarproject.bramble.mailbox.MailboxApiCaller.MAX_RETRY_INTERVAL_MS; +import static org.briarproject.bramble.mailbox.MailboxApiCaller.MIN_RETRY_INTERVAL_MS; + +public class MailboxApiCallerImplTest extends BrambleMockTestCase { + + private final TaskScheduler taskScheduler = + context.mock(TaskScheduler.class); + private final Executor ioExecutor = context.mock(Executor.class); + private final BooleanSupplier supplier = + context.mock(BooleanSupplier.class); + private final Cancellable scheduledTask = context.mock(Cancellable.class); + + private final MailboxApiCallerImpl caller = + new MailboxApiCallerImpl(taskScheduler, ioExecutor); + + @Test + public void testSubmitsTaskImmediately() { + // Calling retryWithBackoff() should schedule the first try immediately + AtomicReference runnable = new AtomicReference<>(null); + context.checking(new Expectations() {{ + oneOf(ioExecutor).execute(with(any(Runnable.class))); + will(new CaptureArgumentAction<>(runnable, Runnable.class, 0)); + }}); + + caller.retryWithBackoff(supplier); + + // When the task runs, the supplier should be called. The supplier + // returns false, so no retries should be scheduled + context.checking(new Expectations() {{ + oneOf(supplier).get(); + will(returnValue(false)); + }}); + + runnable.get().run(); + } + + @Test + public void testDoesNotRetryTaskIfCancelled() { + // Calling retryWithBackoff() should schedule the first try immediately + AtomicReference runnable = new AtomicReference<>(null); + context.checking(new Expectations() {{ + oneOf(ioExecutor).execute(with(any(Runnable.class))); + will(new CaptureArgumentAction<>(runnable, Runnable.class, 0)); + }}); + + Cancellable returned = caller.retryWithBackoff(supplier); + + // When the task runs, the supplier should be called. The supplier + // returns true, so a retry should be scheduled + context.checking(new Expectations() {{ + oneOf(supplier).get(); + will(returnValue(true)); + oneOf(taskScheduler).schedule(with(any(Runnable.class)), + with(ioExecutor), with(MIN_RETRY_INTERVAL_MS), + with(MILLISECONDS)); + will(new DoAllAction( + new CaptureArgumentAction<>(runnable, Runnable.class, 0), + returnValue(scheduledTask) + )); + }}); + + runnable.get().run(); + + // When the Cancellable returned by retryWithBackoff() is cancelled, + // the scheduled task should be cancelled + context.checking(new Expectations() {{ + oneOf(scheduledTask).cancel(); + }}); + + returned.cancel(); + + // Cancelling again should have no effect + returned.cancel(); + + // If the scheduled task runs anyway (cancellation came too late), + // the supplier should not be called and no further tries should be + // scheduled + runnable.get().run(); + } + + @Test + public void testDoublesRetryIntervalUntilMaximumIsReached() { + // The expected retry intervals increase from the min to the max + List expectedIntervals = new ArrayList<>(); + for (long interval = MIN_RETRY_INTERVAL_MS; + interval <= MAX_RETRY_INTERVAL_MS; interval *= 2) { + expectedIntervals.add(interval); + } + // Once the interval reaches the maximum it should be capped + expectedIntervals.add(MAX_RETRY_INTERVAL_MS); + expectedIntervals.add(MAX_RETRY_INTERVAL_MS); + + // Calling retryWithBackoff() should schedule the first try immediately + AtomicReference runnable = new AtomicReference<>(null); + context.checking(new Expectations() {{ + oneOf(ioExecutor).execute(with(any(Runnable.class))); + will(new CaptureArgumentAction<>(runnable, Runnable.class, 0)); + }}); + + caller.retryWithBackoff(supplier); + + // Each time the task runs, the supplier returns true, so a retry + // should be scheduled with a longer interval + for (long interval : expectedIntervals) { + context.checking(new Expectations() {{ + oneOf(supplier).get(); + will(returnValue(true)); + oneOf(taskScheduler).schedule(with(any(Runnable.class)), + with(ioExecutor), with(interval), with(MILLISECONDS)); + will(new DoAllAction( + new CaptureArgumentAction<>( + runnable, Runnable.class, 0), + returnValue(scheduledTask) + )); + }}); + + runnable.get().run(); + } + } + + // Reify the generic type to mollify jMock + private interface BooleanSupplier extends Supplier { + } +} 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 82f58436b..b06539c59 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 @@ -1,5 +1,6 @@ package org.briarproject.bramble.plugin; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.connection.ConnectionManager; import org.briarproject.bramble.api.connection.ConnectionRegistry; import org.briarproject.bramble.api.contact.ContactId; @@ -20,7 +21,6 @@ import org.briarproject.bramble.api.properties.TransportProperties; import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler; -import org.briarproject.bramble.api.system.TaskScheduler.Cancellable; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.ImmediateExecutor; import org.briarproject.bramble.test.RunAction; 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 a2a51146a..76c7bda5a 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 @@ -1,5 +1,6 @@ package org.briarproject.bramble.rendezvous; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.connection.ConnectionManager; import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactState; @@ -27,7 +28,6 @@ import org.briarproject.bramble.api.rendezvous.event.RendezvousConnectionOpenedE import org.briarproject.bramble.api.rendezvous.event.RendezvousPollEvent; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler; -import org.briarproject.bramble.api.system.TaskScheduler.Cancellable; import org.briarproject.bramble.test.BrambleMockTestCase; import org.briarproject.bramble.test.CaptureArgumentAction; import org.briarproject.bramble.test.DbExpectations; diff --git a/bramble-core/src/test/java/org/briarproject/bramble/system/TestTaskScheduler.java b/bramble-core/src/test/java/org/briarproject/bramble/system/TestTaskScheduler.java index 55d753e22..541ba57a9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/system/TestTaskScheduler.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/system/TestTaskScheduler.java @@ -1,5 +1,6 @@ package org.briarproject.bramble.system; +import org.briarproject.bramble.api.Cancellable; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler;