From 933a68558525ae634c8f78113f9ce02adf7146de Mon Sep 17 00:00:00 2001
From: Samuel Holland <samuel@sholland.org>
Date: Tue, 9 Jan 2018 10:03:06 -0600
Subject: model: Chain completions to avoid race conditions

Otherwise getConfigAsync().thenCompose(x -> setState()) would be unsafe.

This reverts commit a6595a273afd50524cc66765c6bfbdcc34cb12e4.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 .../java/com/wireguard/android/model/Tunnel.java   |  9 ++++--
 .../com/wireguard/android/model/TunnelManager.java | 32 ++++++++--------------
 2 files changed, 18 insertions(+), 23 deletions(-)

(limited to 'app/src/main/java/com')

diff --git a/app/src/main/java/com/wireguard/android/model/Tunnel.java b/app/src/main/java/com/wireguard/android/model/Tunnel.java
index 67c5144e..7977d16a 100644
--- a/app/src/main/java/com/wireguard/android/model/Tunnel.java
+++ b/app/src/main/java/com/wireguard/android/model/Tunnel.java
@@ -91,21 +91,24 @@ public class Tunnel extends BaseObservable implements Keyed<String> {
         return CompletableFuture.completedFuture(statistics);
     }
 
-    void onConfigChanged(final Config config) {
+    Config onConfigChanged(final Config config) {
         this.config = config;
         notifyPropertyChanged(BR.config);
+        return config;
     }
 
-    void onStateChanged(final State state) {
+    State onStateChanged(final State state) {
         if (state != State.UP)
             onStatisticsChanged(null);
         this.state = state;
         notifyPropertyChanged(BR.state);
+        return state;
     }
 
-    void onStatisticsChanged(final Statistics statistics) {
+    Statistics onStatisticsChanged(final Statistics statistics) {
         this.statistics = statistics;
         notifyPropertyChanged(BR.statistics);
+        return statistics;
     }
 
     public CompletionStage<Tunnel> rename(@NonNull final String name) {
diff --git a/app/src/main/java/com/wireguard/android/model/TunnelManager.java b/app/src/main/java/com/wireguard/android/model/TunnelManager.java
index 1916ab74..4c053e9a 100644
--- a/app/src/main/java/com/wireguard/android/model/TunnelManager.java
+++ b/app/src/main/java/com/wireguard/android/model/TunnelManager.java
@@ -110,24 +110,18 @@ public final class TunnelManager extends BaseObservable {
     }
 
     CompletionStage<Config> getTunnelConfig(final Tunnel tunnel) {
-        final CompletionStage<Config> completion =
-                asyncWorker.supplyAsync(() -> configStore.load(tunnel.getName()));
-        completion.thenAccept(tunnel::onConfigChanged);
-        return completion;
+        return asyncWorker.supplyAsync(() -> configStore.load(tunnel.getName()))
+                .thenApply(tunnel::onConfigChanged);
     }
 
     CompletionStage<State> getTunnelState(final Tunnel tunnel) {
-        final CompletionStage<State> completion =
-                asyncWorker.supplyAsync(() -> backend.getState(tunnel));
-        completion.thenAccept(tunnel::onStateChanged);
-        return completion;
+        return asyncWorker.supplyAsync(() -> backend.getState(tunnel))
+                .thenApply(tunnel::onStateChanged);
     }
 
     CompletionStage<Statistics> getTunnelStatistics(final Tunnel tunnel) {
-        final CompletionStage<Statistics> completion =
-                asyncWorker.supplyAsync(() -> backend.getStatistics(tunnel));
-        completion.thenAccept(tunnel::onStatisticsChanged);
-        return completion;
+        return asyncWorker.supplyAsync(() -> backend.getStatistics(tunnel))
+                .thenApply(tunnel::onStatisticsChanged);
     }
 
     public ObservableKeyedList<String, Tunnel> getTunnels() {
@@ -227,23 +221,21 @@ public final class TunnelManager extends BaseObservable {
     }
 
     CompletionStage<Config> setTunnelConfig(final Tunnel tunnel, final Config config) {
-        final CompletionStage<Config> completion = asyncWorker.supplyAsync(() -> {
+        return asyncWorker.supplyAsync(() -> {
             final Config appliedConfig = backend.applyConfig(tunnel, config);
             return configStore.save(tunnel.getName(), appliedConfig);
-        });
-        completion.thenAccept(tunnel::onConfigChanged);
-        return completion;
+        }).thenApply(tunnel::onConfigChanged);
     }
 
     CompletionStage<State> setTunnelState(final Tunnel tunnel, final State state) {
-        final CompletionStage<State> completion =
-                asyncWorker.supplyAsync(() -> backend.setState(tunnel, state));
-        completion.whenComplete((newState, e) -> {
+        // Ensure the configuration is loaded before trying to use it.
+        return tunnel.getConfigAsync().thenCompose(x ->
+                asyncWorker.supplyAsync(() -> backend.setState(tunnel, state))
+        ).whenComplete((newState, e) -> {
             // Ensure onStateChanged is always called (failure or not), and with the correct state.
             tunnel.onStateChanged(e == null ? newState : tunnel.getState());
             if (e == null && newState == State.UP)
                 setLastUsedTunnel(tunnel);
         });
-        return completion;
     }
 }
-- 
cgit v1.2.3