use command pattern to start/stop tor service, similar to EIP and ProviderAPI service

7 files changed, 217 insertions, 144 deletions

diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/EipSetupObserver.java b/app/src/main/java/se/leap/bitmaskclient/eip/EipSetupObserver.java
index 203885a1..023a1ce1 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/EipSetupObserver.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/EipSetupObserver.java
@@ -44,6 +44,8 @@ import se.leap.bitmaskclient.base.models.ProviderObservable;
 import se.leap.bitmaskclient.base.utils.PreferenceHelper;
 import se.leap.bitmaskclient.providersetup.ProviderAPI;
 import se.leap.bitmaskclient.providersetup.ProviderAPICommand;
+import se.leap.bitmaskclient.tor.TorServiceCommand;
+import se.leap.bitmaskclient.tor.TorServiceConnection;
 import se.leap.bitmaskclient.tor.TorStatusObservable;
 
 import static android.app.Activity.RESULT_CANCELED;
@@ -213,8 +215,7 @@ public class EipSetupObserver extends BroadcastReceiver implements VpnStatus.Sta
             case INCORRECTLY_DOWNLOADED_EIP_SERVICE:
             case INCORRECTLY_DOWNLOADED_VPN_CERTIFICATE:
                 if (TorStatusObservable.getStatus() != OFF) {
-                    Intent stopIntent = new Intent(context, TorService.class);
-                    context.stopService(stopIntent);
+                    TorServiceCommand.stopTorServiceAsync(context);
                 }
                 Log.d(TAG, "PROVIDER NOK - FETCH FAILED");
                 break;
@@ -368,7 +369,7 @@ public class EipSetupObserver extends BroadcastReceiver implements VpnStatus.Sta
         observedProfileFromVpnStatus = null;
         this.changingGateway.set(changingGateway);
         if (TorStatusObservable.getStatus() != OFF) {
-            TorStatusObservable.shutdownTor(context);
+            TorServiceCommand.stopTorServiceAsync(context);
         }
     }
 
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
index fb8d3e85..709ca651 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
@@ -17,37 +17,25 @@
 package se.leap.bitmaskclient.providersetup;
 
 import android.annotation.SuppressLint;
-import android.app.Notification;
-import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
-import android.content.ServiceConnection;
 import android.content.SharedPreferences;
 import android.net.ConnectivityManager;
 import android.net.NetworkCapabilities;
 import android.net.NetworkInfo;
 import android.os.Build;
-import android.os.IBinder;
-import android.util.Log;
 
 import androidx.annotation.NonNull;
 import androidx.core.app.JobIntentService;
 import androidx.localbroadcastmanager.content.LocalBroadcastManager;
 
-import org.torproject.jni.TorService;
+import java.util.concurrent.TimeoutException;
 
-import java.io.Closeable;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.LinkedBlockingQueue;
-
-import se.leap.bitmaskclient.base.utils.PreferenceHelper;
 import se.leap.bitmaskclient.providersetup.connectivity.OkHttpClientGenerator;
-import se.leap.bitmaskclient.tor.ClientTransportPlugin;
-import se.leap.bitmaskclient.tor.TorNotificationManager;
+import se.leap.bitmaskclient.tor.TorServiceCommand;
+import se.leap.bitmaskclient.tor.TorServiceConnection;
 
 import static se.leap.bitmaskclient.base.models.Constants.SHARED_PREFERENCES;
-import static se.leap.bitmaskclient.base.utils.ConfigHelper.ensureNotOnMainThread;
-import static se.leap.bitmaskclient.tor.TorNotificationManager.TOR_SERVICE_NOTIFICATION_ID;
 
 /**
  * Implements HTTP api methods (encapsulated in {{@link ProviderApiManager}})
@@ -140,65 +128,23 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
     }
 
     @Override
-    public void onDestroy() {
-        super.onDestroy();
-        closeTorServiceConnection();
-    }
-
-    private void closeTorServiceConnection() {
-        if (torServiceConnection != null) {
-            torServiceConnection.close();
-            torServiceConnection = null;
-        }
-    }
-
-    @Override
     public void broadcastEvent(Intent intent) {
         LocalBroadcastManager.getInstance(this).sendBroadcast(intent);
     }
 
     @Override
-    public void startTorService() throws InterruptedException, IllegalStateException {
-        initTorServiceConnection(this);
-        Intent torServiceIntent = new Intent(this, TorService.class);
-
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-            Notification notification = TorNotificationManager.buildTorForegroundNotification(getApplicationContext());
-            //noinspection NewApi
-            getApplicationContext().startForegroundService(torServiceIntent);
-            torServiceConnection.torService.startForeground(TOR_SERVICE_NOTIFICATION_ID, notification);
-        } else {
-            getApplicationContext().startService(torServiceIntent);
-        }
+    public boolean startTorService() throws InterruptedException, IllegalStateException, TimeoutException {
+        return TorServiceCommand.startTorService(this, null);
     }
 
     @Override
     public void stopTorService() {
-        closeTorServiceConnection();
-        try {
-            Intent stopIntent = new Intent(this, TorService.class);
-            stopService(stopIntent);
-        } catch (IllegalStateException e) {
-            e.printStackTrace();
-        }
-
+        TorServiceCommand.stopTorService(this);
     }
 
     @Override
     public int getTorHttpTunnelPort() {
-        try {
-            initTorServiceConnection(this);
-            if (torServiceConnection != null) {
-                int tunnelPort = torServiceConnection.torService.getHttpTunnelPort();
-                torServiceConnection.close();
-                torServiceConnection = null;
-                return tunnelPort;
-            }
-        } catch (InterruptedException | IllegalStateException e) {
-            e.printStackTrace();
-        }
-
-        return -1;
+        return TorServiceCommand.getHttpTunnelPort(this);
     }
 
     @Override
@@ -232,73 +178,4 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
         return new ProviderApiManager(preferences, getResources(), clientGenerator, this);
     }
 
-    /**
-     * Assigns a new TorServiceConnection to ProviderAPI's member variable torServiceConnection.
-     * Only one thread at a time can create the service connection, that will be shared between threads
-     *
-     * @throws InterruptedException  thrown if thread gets interrupted
-     * @throws IllegalStateException thrown if this method was not called from a background thread
-     */
-    private void initTorServiceConnection(Context context) throws InterruptedException, IllegalStateException {
-        if (PreferenceHelper.getUseTor(context)) {
-            if (torServiceConnection == null) {
-                Log.d(TAG, "serviceConnection is still null");
-                if (!TorService.hasClientTransportPlugin()) {
-                    TorService.setClientTransportPlugin(new ClientTransportPlugin(context.getApplicationContext()));
-                }
-                torServiceConnection = new TorServiceConnection(context);
-            }
-        }
-    }
-
-    public static class TorServiceConnection implements Closeable {
-        private final Context context;
-        private ServiceConnection serviceConnection;
-        private TorService torService;
-
-        TorServiceConnection(Context context) throws InterruptedException, IllegalStateException {
-            this.context = context;
-            ensureNotOnMainThread(context);
-            Log.d(TAG, "initSynchronizedServiceConnection!");
-            initSynchronizedServiceConnection(context);
-        }
-
-        @Override
-        public void close() {
-            context.unbindService(serviceConnection);
-        }
-
-        private void initSynchronizedServiceConnection(final Context context) throws InterruptedException {
-            final BlockingQueue<TorService> blockingQueue = new LinkedBlockingQueue<>(1);
-            this.serviceConnection = new ServiceConnection() {
-                volatile boolean mConnectedAtLeastOnce = false;
-
-                @Override
-                public void onServiceConnected(ComponentName name, IBinder service) {
-                    if (!mConnectedAtLeastOnce) {
-                        mConnectedAtLeastOnce = true;
-                        try {
-                            TorService.LocalBinder binder = (TorService.LocalBinder) service;
-                            blockingQueue.put(binder.getService());
-                        } catch (InterruptedException e) {
-                            e.printStackTrace();
-                        }
-                    }
-                }
-
-                @Override
-                public void onServiceDisconnected(ComponentName name) {
-                    torService = null;
-                }
-            };
-            Intent intent = new Intent(context, TorService.class);
-            context.bindService(intent, serviceConnection, Context.BIND_AUTO_CREATE);
-            torService = blockingQueue.take();
-        }
-
-        public TorService getService() {
-            return torService;
-        }
-    }
-
 }
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
index 8d626245..e2f98ca4 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
@@ -152,7 +152,7 @@ public abstract class ProviderApiManagerBase {
 
     public interface ProviderApiServiceCallback {
         void broadcastEvent(Intent intent);
-        void startTorService() throws InterruptedException, IllegalStateException;
+        boolean startTorService() throws InterruptedException, IllegalStateException, TimeoutException;
         void stopTorService();
         int getTorHttpTunnelPort();
         boolean hasNetworkConnection();
@@ -314,9 +314,8 @@ public abstract class ProviderApiManagerBase {
 
     protected boolean startTorProxy() throws InterruptedException, IllegalStateException, TimeoutException {
         if (EipStatus.getInstance().isDisconnected() &&
-                PreferenceHelper.getUseTor(preferences)
-        ) {
-            serviceCallback.startTorService();
+                PreferenceHelper.getUseTor(preferences) &&
+            serviceCallback.startTorService()) {
             waitForTorCircuits();
             if (TorStatusObservable.isCancelled()) {
                 throw new InterruptedException("Cancelled Tor setup.");
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java
index a9f773c3..e429f776 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java
@@ -40,6 +40,7 @@ import se.leap.bitmaskclient.providersetup.ProviderDetailActivity;
 import se.leap.bitmaskclient.providersetup.ProviderManager;
 import se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog;
 import se.leap.bitmaskclient.providersetup.ProviderSetupInterface;
+import se.leap.bitmaskclient.tor.TorServiceCommand;
 import se.leap.bitmaskclient.tor.TorStatusObservable;
 
 import static se.leap.bitmaskclient.base.models.Constants.BROADCAST_PROVIDER_API_EVENT;
@@ -174,7 +175,7 @@ public abstract class ProviderSetupBaseActivity extends ConfigWizardBaseActivity
     public void cancelSettingUpProvider(boolean stopTor) {
         if (stopTor && TorStatusObservable.getStatus() != OFF) {
             Log.d(TAG, "SHUTDOWN - cancelSettingUpProvider stopTor:" + stopTor);
-            TorStatusObservable.shutdownTor(this);
+            TorServiceCommand.stopTorServiceAsync(this);
         }
         providerConfigState = PROVIDER_NOT_SET;
         provider = null;
diff --git a/app/src/main/java/se/leap/bitmaskclient/tor/TorServiceCommand.java b/app/src/main/java/se/leap/bitmaskclient/tor/TorServiceCommand.java
new file mode 100644
index 00000000..1946b861
--- /dev/null
+++ b/app/src/main/java/se/leap/bitmaskclient/tor/TorServiceCommand.java
@@ -0,0 +1,123 @@
+package se.leap.bitmaskclient.tor;
+
+import android.app.Notification;
+import android.content.Context;
+import android.content.Intent;
+import android.os.Build;
+import android.util.Log;
+
+import androidx.annotation.WorkerThread;
+
+import org.torproject.jni.TorService;
+
+import java.util.concurrent.TimeoutException;
+
+import se.leap.bitmaskclient.base.utils.PreferenceHelper;
+
+import static se.leap.bitmaskclient.tor.TorNotificationManager.TOR_SERVICE_NOTIFICATION_ID;
+import static se.leap.bitmaskclient.tor.TorStatusObservable.waitUntil;
+
+public class TorServiceCommand {
+
+
+    private static String TAG = TorServiceCommand.class.getSimpleName();
+
+    // we bind the service before starting it as foreground service so that we avoid startForeground related RemoteExceptions
+    @WorkerThread
+    public static boolean startTorService(Context context, String action) throws InterruptedException {
+        Log.d(TAG, "startTorService");
+        try {
+            waitUntil(TorServiceCommand::isNotCancelled, 30);
+        } catch (TimeoutException e) {
+            e.printStackTrace();
+        }
+        TorServiceConnection torServiceConnection = initTorServiceConnection(context);
+        Log.d(TAG, "startTorService foreground: " + (torServiceConnection != null));
+        boolean startedForeground = false;
+        if (torServiceConnection == null) {
+            return startedForeground;
+        }
+
+        try {
+            Intent torServiceIntent = new Intent(context, TorService.class);
+            torServiceIntent.setAction(action);
+
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+                Notification notification = TorNotificationManager.buildTorForegroundNotification(context.getApplicationContext());
+                //noinspection NewApi
+                context.getApplicationContext().startForegroundService(torServiceIntent);
+                torServiceConnection.getService().startForeground(TOR_SERVICE_NOTIFICATION_ID, notification);
+            } else {
+                context.getApplicationContext().startService(torServiceIntent);
+            }
+            startedForeground = true;
+        } catch (IllegalStateException e) {
+            e.printStackTrace();
+        }
+
+        if (torServiceConnection != null) {
+            torServiceConnection.close();
+        }
+
+        return startedForeground;
+    }
+
+    @WorkerThread
+    public static void stopTorService(Context context) {
+        if (TorStatusObservable.getStatus() == TorStatusObservable.TorStatus.OFF) {
+            return;
+        }
+        TorStatusObservable.markCancelled();
+
+        try {
+            Intent torServiceIntent = new Intent(context, TorService.class);
+            torServiceIntent.setAction(TorService.ACTION_STOP);
+
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+                //noinspection NewApi
+                context.getApplicationContext().startService(torServiceIntent);
+            } else {
+                context.getApplicationContext().startService(torServiceIntent);
+            }
+        } catch (IllegalStateException e) {
+            e.printStackTrace();
+        }
+    }
+
+    public static void stopTorServiceAsync(Context context) {
+        TorStatusObservable.markCancelled();
+        new Thread(() -> stopTorService(context)).start();
+    }
+
+    @WorkerThread
+    public static int getHttpTunnelPort(Context context) {
+        try {
+            TorServiceConnection torServiceConnection = initTorServiceConnection(context);
+            if (torServiceConnection != null) {
+                int tunnelPort = torServiceConnection.getService().getHttpTunnelPort();
+                torServiceConnection.close();
+                return tunnelPort;
+            }
+        } catch (InterruptedException | IllegalStateException e) {
+            e.printStackTrace();
+        }
+        return -1;
+    }
+
+    private static boolean isNotCancelled() {
+        return !TorStatusObservable.isCancelled();
+    }
+
+
+    private static TorServiceConnection initTorServiceConnection(Context context) throws InterruptedException, IllegalStateException {
+        Log.d(TAG, "initTorServiceConnection");
+        if (PreferenceHelper.getUseTor(context)) {
+            Log.d(TAG, "serviceConnection is still null");
+            if (!TorService.hasClientTransportPlugin()) {
+                TorService.setClientTransportPlugin(new ClientTransportPlugin(context.getApplicationContext()));
+            }
+            return new TorServiceConnection(context);
+        }
+        return null;
+    }
+}
diff --git a/app/src/main/java/se/leap/bitmaskclient/tor/TorServiceConnection.java b/app/src/main/java/se/leap/bitmaskclient/tor/TorServiceConnection.java
new file mode 100644
index 00000000..0154983a
--- /dev/null
+++ b/app/src/main/java/se/leap/bitmaskclient/tor/TorServiceConnection.java
@@ -0,0 +1,73 @@
+package se.leap.bitmaskclient.tor;
+
+import android.content.ComponentName;
+import android.content.Context;
+import android.content.Intent;
+import android.content.ServiceConnection;
+import android.os.IBinder;
+import android.util.Log;
+
+import androidx.annotation.WorkerThread;
+
+import org.torproject.jni.TorService;
+
+import java.io.Closeable;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+import se.leap.bitmaskclient.providersetup.ProviderAPI;
+
+import static se.leap.bitmaskclient.base.utils.ConfigHelper.ensureNotOnMainThread;
+
+public class TorServiceConnection implements Closeable {
+    private static final String TAG = TorServiceConnection.class.getSimpleName();
+    private final Context context;
+    private ServiceConnection serviceConnection;
+    private TorService torService;
+
+    @WorkerThread
+    public TorServiceConnection(Context context) throws InterruptedException, IllegalStateException {
+        this.context = context;
+        ensureNotOnMainThread(context);
+        initSynchronizedServiceConnection(context);
+    }
+
+    @Override
+    public void close() {
+        context.unbindService(serviceConnection);
+    }
+
+    private void initSynchronizedServiceConnection(final Context context) throws InterruptedException {
+        Log.d(TAG, "initSynchronizedServiceConnection");
+        final BlockingQueue<TorService> blockingQueue = new LinkedBlockingQueue<>(1);
+        this.serviceConnection = new ServiceConnection() {
+            volatile boolean mConnectedAtLeastOnce = false;
+
+            @Override
+            public void onServiceConnected(ComponentName name, IBinder service) {
+                if (!mConnectedAtLeastOnce) {
+                    mConnectedAtLeastOnce = true;
+                    Log.d(TAG, "onServiceConnected");
+                    try {
+                        TorService.LocalBinder binder = (TorService.LocalBinder) service;
+                        blockingQueue.put(binder.getService());
+                    } catch (InterruptedException e) {
+                        e.printStackTrace();
+                    }
+                }
+            }
+
+            @Override
+            public void onServiceDisconnected(ComponentName name) {
+                torService = null;
+            }
+        };
+        Intent intent = new Intent(context, TorService.class);
+        context.bindService(intent, serviceConnection, Context.BIND_AUTO_CREATE);
+        torService = blockingQueue.take();
+    }
+
+    public TorService getService() {
+        return torService;
+    }
+}
diff --git a/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java b/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java
index ade22e6b..d426d51e 100644
--- a/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java
+++ b/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java
@@ -261,12 +261,11 @@ public class TorStatusObservable extends Observable {
         return "";
     }
 
-    public static void shutdownTor(Context context) {
-        getInstance().cancelled = true;
-        getInstance().notifyObservers();
-
-        Intent intent = new Intent(context, TorService.class);
-        context.stopService(intent);
+    public static void markCancelled() {
+        if (!getInstance().cancelled) {
+            getInstance().cancelled = true;
+            getInstance().notifyObservers();
+        }
     }
 
     public static boolean isCancelled() {