Simplify AdbServerImpl lifecycle

It no longer attempts to handle service restarts.

Issue: #197

device/src/main/java/name/mlopatkin/andlogview/device/AdbManager.java

@@ -27,35 +27,25 @@
  * The implementation is thread-safe.
  */
 public interface AdbManager {
-
-    /**
-     * Updates the current location of the ADB executable. If there is an active {@link AdbServer} then it is
-     * restarted. This method may block for a significant period of time.
-     *
-     * @param adbLocation the new location of the ADB executable
-     * @throws AdbException if the server cannot be started in the new location
-     */
-    void setAdbLocation(AdbLocation adbLocation) throws AdbException;
-
     /**
      * Ensures that the ADB server is running and sets up the connection. If the server isn't running then it is
-     * started. If the running server doesn't match the configured executable then it is killed and a new matching one
-     * os started. This method may block for a significant period of time.
+     * started, otherwise the running server is stopped and a new one starts. This method may block for a significant
+     * period of time.
      *
+     * @param adbLocation the ADB location to use when starting server
      * @return the server
      * @throws AdbException if the server cannot be started
      */
-    AdbServer startServer() throws AdbException;
+    AdbServer startServer(AdbLocation adbLocation) throws AdbException;
 
     /**
      * Creates an instance of the AdbManager but doesn't initialize the DDMLIB yet.
      *
      * @param atExitManager the {@link AtExitManager} to register cleanup hooks
      * @param ioExecutor the executor for non-CPU intensive blocking background work
-     * @param initialAdbLocation the ADB location to use when starting server
      * @return the AdbManager
      */
-    static AdbManager create(AtExitManager atExitManager, Executor ioExecutor, AdbLocation initialAdbLocation) {
-        return new AdbManagerImpl(atExitManager, ioExecutor, initialAdbLocation);
+    static AdbManager create(AtExitManager atExitManager, Executor ioExecutor) {
+        return new AdbManagerImpl(atExitManager, ioExecutor);
     }
 }

device/src/main/java/name/mlopatkin/andlogview/device/AdbManagerImpl.java

@@ -40,41 +40,27 @@ class AdbManagerImpl implements AdbManager {
 
     @GuardedBy("lock")
     private boolean initialized;
-    private volatile @Nullable AdbServerImpl server;
-
     @GuardedBy("lock")
-    private AdbLocation adbLocation;
+    private @Nullable AdbServerImpl server;
 
-    AdbManagerImpl(AtExitManager atExitManager, Executor ioExecutor, AdbLocation initialAdbLocation) {
+    AdbManagerImpl(AtExitManager atExitManager, Executor ioExecutor) {
         this.atExitManager = atExitManager;
         this.ioExecutor = ioExecutor;
-        this.adbLocation = initialAdbLocation;
-    }
-
-    @Override
-    public void setAdbLocation(AdbLocation adbLocation) throws AdbException {
-        synchronized (lock) {
-            this.adbLocation = adbLocation;
-            AdbServerImpl theServer = server;
-            if (theServer != null) {
-                theServer.updateConnection(adbLocation);
-            }
-        }
     }
 
     @Override
-    public AdbServer startServer() throws AdbException {
+    public AdbServer startServer(AdbLocation adbLocation) throws AdbException {
         synchronized (lock) {
-            AdbServerImpl theServer = server;
+            var theServer = server;
             if (theServer != null) {
-                return theServer;
+                theServer.stop();
             }
-            return server = createServerLocked();
+            return server = createServerLocked(adbLocation);
         }
     }
 
     @GuardedBy("lock")
-    private AdbServerImpl createServerLocked() throws AdbException {
+    private AdbServerImpl createServerLocked(AdbLocation adbLocation) throws AdbException {
         initIfNeededLocked();
         return new AdbServerImpl(adbLocation, ioExecutor);
     }
@@ -103,6 +89,11 @@ private void initializeDdmlibLoggingLocked() {
     private void terminate() {
         synchronized (lock) {
             logger.info("Tear down DDMLIB");
+            var theServer = server;
+            if (theServer != null) {
+                logger.info("Stop ADB server");
+                theServer.stop();
+            }
             AndroidDebugBridge.terminate();
         }
     }

device/src/main/java/name/mlopatkin/andlogview/device/AdbServerImpl.java

@@ -22,7 +22,6 @@
 
 import com.android.ddmlib.AndroidDebugBridge;
 import com.android.ddmlib.IDevice;
-import com.google.errorprone.annotations.concurrent.GuardedBy;
 
 import org.apache.log4j.Logger;
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -36,31 +35,20 @@ class AdbServerImpl implements AdbServer, AdbFacade {
     // server instance. Currently, this only happens when someone changes the path to the ADB executable.
     private static final Logger logger = Logger.getLogger(AdbServerImpl.class);
 
-    private final Object lock = new Object();
-
-    @GuardedBy("lock")
-    private AndroidDebugBridge currentBridge;
+    private final AndroidDebugBridge currentBridge;
     private final LazyInstance<DispatchingDeviceList> dispatchingDeviceList;
 
     public AdbServerImpl(AdbLocation adbLocation, Executor ioExecutor) throws AdbException {
         dispatchingDeviceList =
                 lazy(() -> DispatchingDeviceList.create(this, new DeviceProvisionerImpl(this, ioExecutor)));
-        synchronized (lock) {
-            currentBridge = createBridge(adbLocation);
-        }
+        currentBridge = createBridge(adbLocation);
     }
 
     @Override
     public AdbDeviceList getDeviceList(Executor listenerExecutor) {
         return new AdbDeviceListImpl(dispatchingDeviceList.get(), listenerExecutor);
     }
 
-    public void updateConnection(AdbLocation adbLocation) throws AdbException {
-        synchronized (lock) {
-            currentBridge = createBridge(adbLocation);
-        }
-    }
-
     private static AndroidDebugBridge createBridge(AdbLocation adbLocation) throws AdbException {
         logger.info("Starting ADB server");
         File adbExecutable = adbLocation.getExecutable().orElseThrow(() -> new AdbException("ADB location is invalid"));
@@ -88,9 +76,8 @@ private static boolean isBridgeReady(AndroidDebugBridge adb) {
     }
 
     private AndroidDebugBridge getBridge() {
-        synchronized (lock) {
-            return currentBridge;
-        }
+        return currentBridge;
+
     }
 
     @Override
@@ -107,4 +94,8 @@ public void addDeviceChangeListener(AndroidDebugBridge.IDeviceChangeListener dev
     public void removeDeviceChangeListener(AndroidDebugBridge.IDeviceChangeListener deviceChangeListener) {
         AndroidDebugBridge.removeDeviceChangeListener(deviceChangeListener);
     }
+
+    public void stop() {
+        // Do nothing for now.
+    }
 }

src/name/mlopatkin/andlogview/DeviceModule.java

@@ -18,7 +18,6 @@
 
 import name.mlopatkin.andlogview.base.AtExitManager;
 import name.mlopatkin.andlogview.device.AdbManager;
-import name.mlopatkin.andlogview.preferences.AdbConfigurationPref;
 
 import dagger.Module;
 import dagger.Provides;
@@ -35,8 +34,8 @@ private DeviceModule() {
 
     @Provides
     @Singleton
-    static AdbManager getAdbManager(AtExitManager atExitManager, @Named(AppExecutors.FILE_EXECUTOR) Executor ioExecutor,
-            AdbConfigurationPref adbLocation) {
-        return AdbManager.create(atExitManager, ioExecutor, adbLocation);
+    static AdbManager getAdbManager(AtExitManager atExitManager,
+            @Named(AppExecutors.FILE_EXECUTOR) Executor ioExecutor) {
+        return AdbManager.create(atExitManager, ioExecutor);
     }
 }

src/name/mlopatkin/andlogview/ui/device/AdbServicesBridge.java

@@ -101,10 +101,9 @@ private CompletableFuture<AdbServices> initAdbAsync() {
         assert adbSubcomponent == null;
         Stopwatch stopwatch = Stopwatch.createStarted();
 
-        final var result = adbSubcomponent = runAsync(() -> {
-            adbManager.setAdbLocation(adbConfigurationPref);
-            return adbManager.startServer();
-        }, adbInitExecutor).thenApplyAsync(adbSubcomponentFactory::build, uiExecutor);
+        final var result = adbSubcomponent =
+                runAsync(() -> adbManager.startServer(adbConfigurationPref), adbInitExecutor)
+                        .thenApplyAsync(adbSubcomponentFactory::build, uiExecutor);
 
         if (!result.isDone()) {
             // This happens always unless direct executors are used.

test/name/mlopatkin/andlogview/ui/device/AdbServicesBridgeTest.java

@@ -196,7 +196,7 @@ private void drainExecutors(TestExecutor... executor) {
     }
 
     private void whenServerFailsToStart() throws AdbException {
-        when(adbManager.startServer()).thenThrow(new AdbException("Failed to create server"));
+        when(adbManager.startServer(any())).thenThrow(new AdbException("Failed to create server"));
     }
 
     private static void ensureCompleted(Future<?> f) {