Clean up service overlays on interruption (#2567)

- Release default and SurfaceControl overlays during service shutdown
- Guard queued overlay operations by service window generation
- Tear down overlay windows during service unbind as well as destroy

🤖 Auto-generated

Co-authored-by: Owen McGirr <o.a.mcgirr@gmail.com>

Author: enaboapps

app/src/main/java/com/enaboapps/switchify/service/core/SwitchifyAccessibilityService.kt

@@ -321,6 +321,7 @@ class SwitchifyAccessibilityService : AccessibilityService(), LifecycleOwner,
             eventPipeline.stop()
         }
         ServiceCore.cleanup()
+        SwitchifyAccessibilityWindow.instance.onServiceDestroy()
         PcMouseRepeatManager.instance.clearServiceState()
         GestureRepeatManager.instance.clearServiceState()
         GestureLockManager.instance.clearServiceState()

app/src/main/java/com/enaboapps/switchify/service/window/SwitchifyAccessibilityWindow.kt

@@ -27,6 +27,8 @@ import com.enaboapps.switchify.service.window.overlay.OverlayTargets
 import com.enaboapps.switchify.service.window.overlay.OverlayHandle
 import com.enaboapps.switchify.service.window.overlay.SurfaceControlOverlayBackend
 import com.enaboapps.switchify.service.window.overlay.SwitchifyOverlayHost
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
 
 /**
  * This class manages the window for the Switchify accessibility service.
@@ -43,9 +45,12 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
     private val mainHandler = Handler(Looper.getMainLooper())
     private var screenWatcher: ScreenWatcher? = null
     private var isVisible = false
+    private var windowGeneration = 0
+    private var acceptingOverlayOperations = false
 
     companion object {
         private const val TAG = "SwitchifyAccessibilityWindow"
+        private const val SHUTDOWN_CLEANUP_TIMEOUT_MS = 500L
         val instance: SwitchifyAccessibilityWindow by lazy {
             SwitchifyAccessibilityWindow()
         }
@@ -65,15 +70,17 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
      */
     fun setup(context: Context) {
         try {
-            // Clean up previous state if it exists
-            cleanup()
+            acceptingOverlayOperations = false
+            windowGeneration += 1
+            cleanupOnMainBlocking()
 
             this.context = context
             windowManager = context.getSystemService(Context.WINDOW_SERVICE) as WindowManager
             surfaceControlBackend = (context as? AccessibilityService)?.let { service ->
                 SurfaceControlOverlayBackend(service) { baseLayout?.windowToken }
             }
             createBaseLayout()
+            acceptingOverlayOperations = true
             registerScreenWatcher()
             ServiceMessageHUD.instance.setup(context.applicationContext)
             MenuHighlightHud.instance.setup(context.applicationContext)
@@ -154,12 +161,11 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
      * Shows the window.
      */
     fun show() {
-        mainHandler.post {
+        postIfCurrentGeneration {
             try {
-                // Avoid adding duplicate views
                 if (isVisible) {
                     Log.d(TAG, "Window already visible, skipping show()")
-                    return@post
+                    return@postIfCurrentGeneration
                 }
 
                 val params = WindowManager.LayoutParams(
@@ -190,7 +196,7 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
      * Hides the window without cleaning up views.
      */
     fun hide() {
-        mainHandler.post {
+        postIfCurrentGeneration {
             try {
                 if (isVisible && baseLayout != null) {
                     windowManager?.removeView(baseLayout)
@@ -208,41 +214,26 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
      * Cleans up the window and its resources.
      */
     fun cleanup() {
-        mainHandler.post {
-            try {
-                for (i in 0 until (baseLayout?.childCount ?: 0)) {
-                    val child = baseLayout?.getChildAt(i)
-                    if (child is ViewGroup) {
-                        child.removeAllViews()
-                    }
-                }
-
-                if (isVisible && baseLayout != null) {
-                    windowManager?.removeView(baseLayout)
-                    isVisible = false
-                }
-
-                // Clear all children from baseLayout
-                baseLayout?.removeAllViews()
-                releaseSurfaceOverlays()
-            } catch (e: Exception) {
-                Log.e(TAG, "Error in cleanup: ${e.message}", e)
-            }
+        if (Looper.myLooper() == Looper.getMainLooper()) {
+            if (acceptingOverlayOperations) cleanupNow()
+            return
         }
+        postIfCurrentGeneration { cleanupNow() }
     }
 
     /**
      * Cleans up the window and its resources when the service is destroyed.
      */
     fun onServiceDestroy() {
+        acceptingOverlayOperations = false
         ServiceMessageHUD.instance.dispose()
         MenuHighlightHud.instance.dispose()
         ServiceStartupSplash.instance.dispose()
         MediaPipeBackend.close()
-        cleanup()
-        isVisible = false // Ensure the flag is set to false for the next time the window is created
-        val ctx = getContext() ?: return
-        screenWatcher?.unregister(ctx)
+        cleanupForServiceShutdown()
+        getContext()?.let { ctx ->
+            screenWatcher?.unregister(ctx)
+        }
         screenWatcher = null
         context = null
         windowManager = null
@@ -324,22 +315,22 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
         view: ViewGroup,
         placement: OverlayPlacement
     ) {
-        mainHandler.post {
+        postIfCurrentGeneration {
             try {
                 if (shouldUseSurfaceBackend(target)) {
                     val handle = surfaceControlBackend?.attach(target, view, placement)
                     if (handle != null) {
                         handle.setVisible(isVisible)
                         surfaceOverlayHandles[view] = handle
-                        return@post
+                        return@postIfCurrentGeneration
                     }
                     if (target is OverlayTarget.Window) {
                         Log.w(TAG, "Window overlay unavailable for $target; skipping fallback")
-                        return@post
+                        return@postIfCurrentGeneration
                     }
                     if (!canUseDefaultRootFallback(target)) {
                         Log.w(TAG, "Surface overlay unavailable for $target; no default-root fallback")
-                        return@post
+                        return@postIfCurrentGeneration
                     }
                     Log.w(TAG, "Surface overlay unavailable for $target; falling back to default overlay")
                 }
@@ -469,11 +460,11 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
     }
 
     fun removeView(target: OverlayTarget, view: ViewGroup) {
-        mainHandler.post {
+        postIfCurrentGeneration {
             try {
                 surfaceOverlayHandles.remove(view)?.let { handle ->
                     handle.release()
-                    return@post
+                    return@postIfCurrentGeneration
                 }
                 if (view.parent == baseLayout) {
                     baseLayout?.removeView(view)
@@ -495,11 +486,11 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
     }
 
     override fun removeView(target: OverlayTarget.Display, id: Int) {
-        mainHandler.post {
+        postIfCurrentGeneration {
             try {
                 surfaceOverlayHandles.entries.firstOrNull { it.key.id == id }?.let { entry ->
                     surfaceOverlayHandles.remove(entry.key)?.release()
-                    return@post
+                    return@postIfCurrentGeneration
                 }
                 baseLayout?.findViewById<ViewGroup>(id)?.let { view ->
                     baseLayout?.removeView(view)
@@ -528,6 +519,61 @@ class SwitchifyAccessibilityWindow private constructor() : LifecycleOwner, Saved
             surfaceControlBackend?.canAttach(target) == true
     }
 
+    private fun postIfCurrentGeneration(block: () -> Unit) {
+        val generation = windowGeneration
+        mainHandler.post {
+            if (generation != windowGeneration || !acceptingOverlayOperations) return@post
+            block()
+        }
+    }
+
+    private fun cleanupForServiceShutdown() {
+        acceptingOverlayOperations = false
+        cleanupOnMainBlocking()
+    }
+
+    private fun cleanupOnMainBlocking() {
+        if (Looper.myLooper() == Looper.getMainLooper()) {
+            cleanupNow()
+            return
+        }
+        val latch = CountDownLatch(1)
+        mainHandler.postAtFrontOfQueue {
+            try {
+                cleanupNow()
+            } finally {
+                latch.countDown()
+            }
+        }
+        if (!latch.await(SHUTDOWN_CLEANUP_TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
+            Log.w(TAG, "Timed out waiting for window cleanup")
+        }
+    }
+
+    private fun cleanupNow() {
+        try {
+            releaseSurfaceOverlays()
+            for (i in 0 until (baseLayout?.childCount ?: 0)) {
+                val child = baseLayout?.getChildAt(i)
+                if (child is ViewGroup) {
+                    child.removeAllViews()
+                }
+            }
+            baseLayout?.removeAllViews()
+            baseLayout?.let { layout ->
+                if (isVisible || layout.parent != null) {
+                    windowManager?.removeViewImmediate(layout)
+                }
+            }
+            isVisible = false
+        } catch (e: IllegalArgumentException) {
+            isVisible = false
+            Log.w(TAG, "Window was already removed during cleanup", e)
+        } catch (e: Exception) {
+            Log.e(TAG, "Error in cleanup: ${e.message}", e)
+        }
+    }
+
     private fun releaseSurfaceOverlays() {
         surfaceOverlayHandles.values.forEach { handle ->
             try {

app/src/main/java/com/enaboapps/switchify/service/window/overlay/SurfaceControlOverlayBackend.kt

@@ -196,20 +196,30 @@ class SurfaceControlOverlayBackend(
         private val surfaceControl: SurfaceControl,
         private val viewHost: SurfaceControlViewHost
     ) : OverlayHandle {
+        private var released = false
+
         override fun setVisible(visible: Boolean) {
+            if (released) return
             SurfaceControl.Transaction()
                 .setVisibility(surfaceControl, visible)
                 .apply()
         }
 
         override fun release() {
-            if (view.parent === container) {
-                container.removeView(view)
+            if (released) return
+            released = true
+            try {
+                if (view.parent === container) {
+                    container.removeView(view)
+                }
+                SurfaceControl.Transaction()
+                    .reparent(surfaceControl, null)
+                    .apply()
+            } catch (e: Exception) {
+                Log.e(TAG, "Failed to release SurfaceControl overlay", e)
+            } finally {
+                viewHost.release()
             }
-            SurfaceControl.Transaction()
-                .reparent(surfaceControl, null)
-                .apply()
-            viewHost.release()
         }
     }