From 6a61ed5695f5b3830adcf4d3b468ec05d2dcfda5 Mon Sep 17 00:00:00 2001
From: Jarvis Huang <quic_jarvhuan@quicinc.com>
Date: Wed, 27 Oct 2021 12:10:11 +0800
Subject: [PATCH] ipc/android: Stop runtime service when no clients connected

---
 .../monado/auxiliary/IServiceNotification.kt  |  2 +-
 src/xrt/ipc/android/build.gradle              |  4 +-
 .../ipc/android/src/main/AndroidManifest.xml  |  1 -
 .../org/freedesktop/monado/ipc/Client.java    | 13 ----
 .../freedesktop/monado/ipc/MonadoImpl.java    | 44 +++++------
 .../freedesktop/monado/ipc/MonadoService.kt   | 62 +++++++++------
 .../org/freedesktop/monado/ipc/Watchdog.kt    | 78 +++++++++++++++++++
 .../android_common/ServiceNotificationImpl.kt |  8 +-
 .../targets/service-lib/service_target.cpp    | 78 +++++++++++++------
 9 files changed, 192 insertions(+), 98 deletions(-)
 create mode 100644 src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Watchdog.kt

diff --git a/src/xrt/auxiliary/android/src/main/java/org/freedesktop/monado/auxiliary/IServiceNotification.kt b/src/xrt/auxiliary/android/src/main/java/org/freedesktop/monado/auxiliary/IServiceNotification.kt
index 654366fa1..6ea49891d 100644
--- a/src/xrt/auxiliary/android/src/main/java/org/freedesktop/monado/auxiliary/IServiceNotification.kt
+++ b/src/xrt/auxiliary/android/src/main/java/org/freedesktop/monado/auxiliary/IServiceNotification.kt
@@ -24,7 +24,7 @@ interface IServiceNotification {
      * Create and return a notification (creating the channel if applicable) that can be used in
      * {@code Service#startForeground()}
      */
-    fun buildNotification(context: Context, pendingShutdownIntent: PendingIntent): Notification
+    fun buildNotification(context: Context): Notification
 
     /**
      * Return the notification ID to use
diff --git a/src/xrt/ipc/android/build.gradle b/src/xrt/ipc/android/build.gradle
index c1ec82a5e..5ebf0c13e 100644
--- a/src/xrt/ipc/android/build.gradle
+++ b/src/xrt/ipc/android/build.gradle
@@ -21,13 +21,11 @@ android {
 
         // Single point of truth for these names.
         def serviceAction = "org.freedesktop.monado.ipc.CONNECT"
-        def shutdownAction = "org.freedesktop.monado.ipc.SHUTDOWN"
         manifestPlaceholders = [
                 serviceActionName : serviceAction,
-                shutdownActionName: shutdownAction
         ]
         buildConfigField("String", "SERVICE_ACTION", "\"${serviceAction}\"")
-        buildConfigField("String", "SHUTDOWN_ACTION", "\"${shutdownAction}\"")
+        buildConfigField("Long", "WATCHDOG_TIMEOUT_MILLISECONDS", "1500L")
     }
 
     buildTypes {
diff --git a/src/xrt/ipc/android/src/main/AndroidManifest.xml b/src/xrt/ipc/android/src/main/AndroidManifest.xml
index 4f8c19ed8..6bb3b14f0 100644
--- a/src/xrt/ipc/android/src/main/AndroidManifest.xml
+++ b/src/xrt/ipc/android/src/main/AndroidManifest.xml
@@ -15,7 +15,6 @@
 
             <intent-filter>
                 <action android:name="${serviceActionName}" />
-                <action android:name="${shutdownActionName}" />
             </intent-filter>
         </service>
     </application>
diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java
index 890a4b7aa..ca4c1e3d9 100644
--- a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java
+++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Client.java
@@ -78,10 +78,6 @@ public class Client implements ServiceConnection {
      * Context of the runtime package
      */
     private Context runtimePackageContext = null;
-    /**
-     * Intent for connecting to service
-     */
-    private Intent intent = null;
     /**
      * Controll system ui visibility
      */
@@ -103,10 +99,6 @@ public class Client implements ServiceConnection {
         if (context != null) {
             context.unbindService(this);
         }
-        if (intent != null) {
-            context.stopService(intent);
-        }
-        intent = null;
 
         if (fd != null) {
             try {
@@ -255,11 +247,6 @@ public class Client implements ServiceConnection {
         Intent intent = new Intent(BuildConfig.SERVICE_ACTION)
                 .setPackage(packageName);
 
-        if (context.startForegroundService(intent) == null) {
-            Log.e(TAG, "startForegroundService: Service " + intent.toString() + " does not exist!");
-            return false;
-        }
-
         if (!bindService(context, intent)) {
             Log.e(TAG,
                     "bindService: Service " + intent.toString() + " could not be found to bind!");
diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java
index 30d7f06f8..fbce141ec 100644
--- a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java
+++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoImpl.java
@@ -42,11 +42,6 @@ public class MonadoImpl extends IMonado.Stub {
         System.loadLibrary("monado-service");
     }
 
-    private final Thread compositorThread = new Thread(
-            this::threadEntry,
-            "CompositorThread");
-    private boolean started = false;
-
     private SurfaceManager surfaceManager;
 
     public MonadoImpl(@NonNull SurfaceManager surfaceManager) {
@@ -55,7 +50,7 @@ public class MonadoImpl extends IMonado.Stub {
             @Override
             public void surfaceCreated(@NonNull SurfaceHolder holder) {
                 Log.i(TAG, "surfaceCreated");
-                nativeAppSurface(holder.getSurface());
+                passAppSurface(holder.getSurface());
             }
 
             @Override
@@ -68,20 +63,8 @@ public class MonadoImpl extends IMonado.Stub {
         });
     }
 
-    private void launchThreadIfNeeded() {
-        synchronized (compositorThread) {
-            if (!started) {
-                compositorThread.start();
-                nativeWaitForServerStartup();
-                started = true;
-            }
-        }
-    }
-
     @Override
     public void connect(@NotNull ParcelFileDescriptor parcelFileDescriptor) {
-        /// @todo launch this thread earlier/elsewhere
-        launchThreadIfNeeded();
         int fd = parcelFileDescriptor.getFd();
         Log.i(TAG, "connect: given fd " + fd);
         if (nativeAddClient(fd) != 0) {
@@ -105,6 +88,12 @@ public class MonadoImpl extends IMonado.Stub {
             return;
         }
         nativeAppSurface(surface);
+        startServerIfNeeded();
+    }
+
+    private void startServerIfNeeded() {
+        nativeStartServer();
+        nativeWaitForServerStartup();
     }
 
     @Override
@@ -119,17 +108,16 @@ public class MonadoImpl extends IMonado.Stub {
         return surfaceManager.canDrawOverlays();
     }
 
-    private void threadEntry() {
-        Log.i(TAG, "threadEntry");
-        nativeThreadEntry();
-        Log.i(TAG, "native thread has exited");
+    public void shutdown() {
+        Log.i(TAG, "shutdown");
+        nativeShutdownServer();
     }
 
     /**
-     * Native thread entry point.
+     * Native method that starts server.
      */
     @SuppressWarnings("JavaJniMissingFunction")
-    private native void nativeThreadEntry();
+    private native void nativeStartServer();
 
     /**
      * Native method that waits until the server reports that it is, in fact, started up.
@@ -167,4 +155,12 @@ public class MonadoImpl extends IMonado.Stub {
      */
     @SuppressWarnings("JavaJniMissingFunction")
     private native int nativeAddClient(int fd);
+
+    /**
+     * Native method that handles shutdown server.
+     *
+     * @return 0 on success; -1 means that server didn't start.
+     */
+    @SuppressWarnings("JavaJniMissingFunction")
+    private native int nativeShutdownServer();
 }
diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt
index 306052d6a..fa319c3e1 100644
--- a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt
+++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/MonadoService.kt
@@ -8,7 +8,6 @@
  */
 package org.freedesktop.monado.ipc
 
-import android.app.PendingIntent
 import android.app.Service
 import android.content.Intent
 import android.content.pm.ServiceInfo
@@ -25,10 +24,10 @@ import javax.inject.Inject
  * This is needed so that the APK can expose the binder service implemented in MonadoImpl.
  */
 @AndroidEntryPoint
-class MonadoService : Service() {
-    private val binder: MonadoImpl by lazy {
-        MonadoImpl(surfaceManager)
-    }
+class MonadoService : Service(), Watchdog.ShutdownListener {
+    private lateinit var binder: MonadoImpl
+
+    private val watchdog = Watchdog(BuildConfig.WATCHDOG_TIMEOUT_MILLISECONDS, this)
 
     @Inject
     lateinit var serviceNotification: IServiceNotification
@@ -39,44 +38,36 @@ class MonadoService : Service() {
         super.onCreate()
 
         surfaceManager = SurfaceManager(this)
+        binder = MonadoImpl(surfaceManager)
+        watchdog.startMonitor()
+
+        // start the service so it could be foregrounded
+        startService(Intent(this, javaClass))
     }
 
     override fun onDestroy() {
         super.onDestroy()
+        Log.d(TAG, "onDestroy")
 
+        binder.shutdown();
+        watchdog.stopMonitor()
         surfaceManager.destroySurface()
     }
 
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
         Log.d(TAG, "onStartCommand")
-        // if this isn't a restart
-        if (intent != null) {
-            when (intent.action) {
-                BuildConfig.SERVICE_ACTION -> handleStart()
-                BuildConfig.SHUTDOWN_ACTION -> handleShutdown()
-            }
-        }
+        handleStart()
         return START_STICKY
     }
 
-    private fun handleShutdown() {
-        stopForeground(true)
-        stopSelf()
-    }
-
     override fun onBind(intent: Intent): IBinder? {
+        Log.d(TAG, "onBind");
+        watchdog.onClientConnected()
         return binder
     }
 
     private fun handleStart() {
-        val pendingShutdownIntent = PendingIntent.getForegroundService(
-            this,
-            0,
-            Intent(BuildConfig.SHUTDOWN_ACTION).setPackage(packageName),
-            0
-        )
-
-        val notification = serviceNotification.buildNotification(this, pendingShutdownIntent)
+        val notification = serviceNotification.buildNotification(this)
 
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
             startForeground(
@@ -92,6 +83,27 @@ class MonadoService : Service() {
         }
     }
 
+    override fun onUnbind(intent: Intent?): Boolean {
+        Log.d(TAG, "onUnbind");
+        watchdog.onClientDisconnected()
+        return true;
+    }
+
+    override fun onRebind(intent: Intent?) {
+        Log.d(TAG, "onRebind");
+        watchdog.onClientConnected()
+    }
+
+    override fun onPrepareShutdown() {
+        Log.d(TAG, "onPrepareShutdown")
+    }
+
+    override fun onShutdown() {
+        Log.d(TAG, "onShutdown")
+        stopForeground(true)
+        stopSelf()
+    }
+
     companion object {
         private const val TAG = "MonadoService"
     }
diff --git a/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Watchdog.kt b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Watchdog.kt
new file mode 100644
index 000000000..6397a7927
--- /dev/null
+++ b/src/xrt/ipc/android/src/main/java/org/freedesktop/monado/ipc/Watchdog.kt
@@ -0,0 +1,78 @@
+// Copyright 2021, Qualcomm Innovation Center, Inc.
+// SPDX-License-Identifier: BSL-1.0
+/*!
+ * @file
+ * @brief  Monitor client connections.
+ * @author Jarvis Huang
+ * @ingroup ipc_android
+ */
+package org.freedesktop.monado.ipc
+
+import android.os.Handler
+import android.os.HandlerThread
+import android.os.Message
+import java.util.concurrent.atomic.AtomicInteger
+
+/**
+ * Client watchdog, to determine whether runtime service should be stopped.
+ */
+class Watchdog(
+    private val shutdownDelayMilliseconds: Long,
+    private val shutdownListener: ShutdownListener
+) {
+    /**
+     * Interface definition for callbacks to be invoked when there's no client connected. Noted that
+     * all the callbacks run on background thread.
+     */
+    interface ShutdownListener {
+        /**
+         * Callback to be invoked when last client disconnected.
+         */
+        fun onPrepareShutdown()
+
+        /**
+         * Callback to be invoked when shutdown delay ended and there's no new client connected.
+         */
+        fun onShutdown()
+    }
+
+    private val clientCount = AtomicInteger(0)
+
+    private lateinit var shutdownHandler: Handler
+
+    private lateinit var shutdownThread: HandlerThread
+
+    fun startMonitor() {
+        shutdownThread = HandlerThread("monado-client-watchdog")
+        shutdownThread.start()
+        shutdownHandler = object : Handler(shutdownThread.looper) {
+            override fun handleMessage(msg: Message) {
+                when (msg.what) {
+                    MSG_SHUTDOWN -> if (clientCount.get() == 0) {
+                        shutdownListener.onShutdown()
+                    }
+                }
+            }
+        }
+    }
+
+    fun stopMonitor() {
+        shutdownThread.quitSafely()
+    }
+
+    fun onClientConnected() {
+        clientCount.incrementAndGet()
+        shutdownHandler.removeMessages(MSG_SHUTDOWN)
+    }
+
+    fun onClientDisconnected() {
+        if (clientCount.decrementAndGet() == 0) {
+            shutdownListener.onPrepareShutdown()
+            shutdownHandler.sendEmptyMessageDelayed(MSG_SHUTDOWN, shutdownDelayMilliseconds)
+        }
+    }
+
+    companion object {
+        private const val MSG_SHUTDOWN = 1000
+    }
+}
diff --git a/src/xrt/targets/android_common/src/main/java/org/freedesktop/monado/android_common/ServiceNotificationImpl.kt b/src/xrt/targets/android_common/src/main/java/org/freedesktop/monado/android_common/ServiceNotificationImpl.kt
index 12e9d307b..bf2692015 100644
--- a/src/xrt/targets/android_common/src/main/java/org/freedesktop/monado/android_common/ServiceNotificationImpl.kt
+++ b/src/xrt/targets/android_common/src/main/java/org/freedesktop/monado/android_common/ServiceNotificationImpl.kt
@@ -70,14 +70,9 @@ class ServiceNotificationImpl @Inject constructor() : IServiceNotification {
      * Create and return a notification (creating the channel if applicable) that can be used in
      * {@code Service#startForeground()}
      */
-    override fun buildNotification(context: Context, pendingShutdownIntent: PendingIntent): Notification {
+    override fun buildNotification(context: Context): Notification {
         createChannel(context)
 
-        val action = Notification.Action.Builder(
-                Icon.createWithResource(context, R.drawable.ic_feathericons_x),
-                context.getString(R.string.notifExitRuntime),
-                pendingShutdownIntent)
-                .build()
         // Make a notification for our foreground service
         // When selected it will open the "About" activity
         val builder = makeNotificationBuilder(context)
@@ -87,7 +82,6 @@ class ServiceNotificationImpl @Inject constructor() : IServiceNotification {
                         R.string.notif_text,
                         nameAndLogoProvider.getLocalizedRuntimeName()))
                 .setShowWhen(false)
-                .addAction(action)
                 .setContentIntent(uiProvider.makeAboutActivityPendingIntent())
 
         // Notification icon is optional
diff --git a/src/xrt/targets/service-lib/service_target.cpp b/src/xrt/targets/service-lib/service_target.cpp
index 30d828bc7..a96c05eeb 100644
--- a/src/xrt/targets/service-lib/service_target.cpp
+++ b/src/xrt/targets/service-lib/service_target.cpp
@@ -20,45 +20,33 @@
 #include <android/native_window_jni.h>
 
 #include "android/android_globals.h"
+
+#include <memory>
 #include <thread>
 
 using wrap::android::view::Surface;
 namespace {
-struct Singleton
+struct IpcServerHelper
 {
 public:
-	static Singleton &
-	instance()
-	{
-		static Singleton singleton{};
-		return singleton;
-	}
-
+	IpcServerHelper() {}
 
 	void
 	waitForStartupComplete()
 	{
-
 		std::unique_lock<std::mutex> lock{running_mutex};
 		running_cond.wait(lock, [&]() { return this->startup_complete; });
 	}
 
-	//! static trampoline for the startup complete callback
-	static void
-	signalStartupComplete()
-	{
-		instance().signalStartupCompleteNonstatic();
-	}
-
-private:
 	void
-	signalStartupCompleteNonstatic()
+	signalStartupComplete()
 	{
 		std::unique_lock<std::mutex> lock{running_mutex};
 		startup_complete = true;
 		running_cond.notify_all();
 	}
-	Singleton() {}
+
+private:
 	//! Mutex for starting thread
 	std::mutex running_mutex;
 
@@ -69,27 +57,44 @@ private:
 } // namespace
 
 static struct ipc_server *server = NULL;
+static IpcServerHelper *helper = nullptr;
+static std::unique_ptr<std::thread> server_thread{};
+static std::mutex server_thread_mutex;
 
 static void
 signalStartupCompleteTrampoline(void *data)
 {
-	static_cast<Singleton *>(data)->signalStartupComplete();
+	static_cast<IpcServerHelper *>(data)->signalStartupComplete();
 }
 
 extern "C" void
-Java_org_freedesktop_monado_ipc_MonadoImpl_nativeThreadEntry(JNIEnv *env, jobject thiz)
+Java_org_freedesktop_monado_ipc_MonadoImpl_nativeStartServer(JNIEnv *env, jobject thiz)
 {
 	jni::init(env);
 	jni::Object monadoImpl(thiz);
 	U_LOG_D("service: Called nativeThreadEntry");
-	auto &singleton = Singleton::instance();
-	ipc_server_main_android(&server, signalStartupCompleteTrampoline, &singleton);
+
+	{
+		// Start IPC server
+		std::unique_lock lock(server_thread_mutex);
+		if (!server && !server_thread) {
+			helper = new IpcServerHelper();
+			server_thread = std::make_unique<std::thread>(
+			    []() { ipc_server_main_android(&server, signalStartupCompleteTrampoline, helper); });
+			helper->waitForStartupComplete();
+		}
+	}
 }
 
 extern "C" JNIEXPORT void JNICALL
 Java_org_freedesktop_monado_ipc_MonadoImpl_nativeWaitForServerStartup(JNIEnv *env, jobject thiz)
 {
-	Singleton::instance().waitForStartupComplete();
+	if (server == nullptr) {
+		// Should not happen.
+		U_LOG_E("service: nativeWaitForServerStartup called before service started up!");
+		return;
+	}
+	helper->waitForStartupComplete();
 }
 
 extern "C" JNIEXPORT jint JNICALL
@@ -118,3 +123,28 @@ Java_org_freedesktop_monado_ipc_MonadoImpl_nativeAppSurface(JNIEnv *env, jobject
 	android_globals_store_window((struct _ANativeWindow *)nativeWindow);
 	U_LOG_D("Stored ANativeWindow: %p", (void *)nativeWindow);
 }
+
+extern "C" JNIEXPORT jint JNICALL
+Java_org_freedesktop_monado_ipc_MonadoImpl_nativeShutdownServer(JNIEnv *env, jobject thiz)
+{
+	jni::init(env);
+	jni::Object monadoImpl(thiz);
+	if (server == nullptr || !server_thread) {
+		// Should not happen.
+		U_LOG_E("service: nativeShutdownServer called before service started up!");
+		return -1;
+	}
+
+	{
+		// Wait until IPC server stop
+		std::unique_lock lock(server_thread_mutex);
+		ipc_server_handle_shutdown_signal(server);
+		server_thread->join();
+		server_thread.reset(nullptr);
+		delete helper;
+		helper = nullptr;
+		server = NULL;
+	}
+
+	return 0;
+}