Merge "Remove legacy debian.log file" into main
diff --git a/android/TerminalApp/java/com/android/virtualization/terminal/ConfigJson.kt b/android/TerminalApp/java/com/android/virtualization/terminal/ConfigJson.kt
index 1fd58cd..5d22790 100644
--- a/android/TerminalApp/java/com/android/virtualization/terminal/ConfigJson.kt
+++ b/android/TerminalApp/java/com/android/virtualization/terminal/ConfigJson.kt
@@ -29,7 +29,6 @@
 import com.android.virtualization.terminal.ConfigJson.InputJson
 import com.android.virtualization.terminal.ConfigJson.PartitionJson
 import com.android.virtualization.terminal.ConfigJson.SharedPathJson
-import com.android.virtualization.terminal.InstalledImage.Companion.getDefault
 import com.google.gson.Gson
 import com.google.gson.annotations.SerializedName
 import java.io.BufferedReader
@@ -313,7 +312,7 @@
         private fun replaceKeywords(r: Reader, context: Context): String {
             val rules: Map<String, String> =
                 mapOf(
-                    "\\\$PAYLOAD_DIR" to getDefault(context).installDir.toString(),
+                    "\\\$PAYLOAD_DIR" to InstalledImage.getDefault(context).installDir.toString(),
                     "\\\$USER_ID" to context.userId.toString(),
                     "\\\$PACKAGE_NAME" to context.getPackageName(),
                     "\\\$APP_DATA_DIR" to context.getDataDir().toString(),
diff --git a/android/TerminalApp/java/com/android/virtualization/terminal/DebianServiceImpl.kt b/android/TerminalApp/java/com/android/virtualization/terminal/DebianServiceImpl.kt
index e81be7f..2c52283 100644
--- a/android/TerminalApp/java/com/android/virtualization/terminal/DebianServiceImpl.kt
+++ b/android/TerminalApp/java/com/android/virtualization/terminal/DebianServiceImpl.kt
@@ -21,7 +21,6 @@
 import com.android.internal.annotations.GuardedBy
 import com.android.system.virtualmachine.flags.Flags
 import com.android.virtualization.terminal.MainActivity.Companion.TAG
-import com.android.virtualization.terminal.PortsStateManager.Companion.getInstance
 import com.android.virtualization.terminal.proto.DebianServiceGrpc.DebianServiceImplBase
 import com.android.virtualization.terminal.proto.ForwardingRequestItem
 import com.android.virtualization.terminal.proto.QueueOpeningRequest
@@ -35,7 +34,7 @@
 import io.grpc.stub.StreamObserver
 
 internal class DebianServiceImpl(context: Context) : DebianServiceImplBase() {
-    private val portsStateManager: PortsStateManager = getInstance(context)
+    private val portsStateManager = PortsStateManager.getInstance(context)
     private var portsStateListener: PortsStateManager.Listener? = null
     private var shutdownRunnable: Runnable? = null
     private val mLock = Object()
diff --git a/android/TerminalApp/java/com/android/virtualization/terminal/MainActivity.kt b/android/TerminalApp/java/com/android/virtualization/terminal/MainActivity.kt
index 390ae00..487b7e8 100644
--- a/android/TerminalApp/java/com/android/virtualization/terminal/MainActivity.kt
+++ b/android/TerminalApp/java/com/android/virtualization/terminal/MainActivity.kt
@@ -15,6 +15,7 @@
  */
 package com.android.virtualization.terminal
 
+import android.app.ForegroundServiceStartNotAllowedException
 import android.app.Notification
 import android.app.PendingIntent
 import android.content.Context
@@ -49,10 +50,6 @@
 import com.android.internal.annotations.VisibleForTesting
 import com.android.microdroid.test.common.DeviceProperties
 import com.android.system.virtualmachine.flags.Flags.terminalGuiSupport
-import com.android.virtualization.terminal.ErrorActivity.Companion.start
-import com.android.virtualization.terminal.InstalledImage.Companion.getDefault
-import com.android.virtualization.terminal.VmLauncherService.Companion.run
-import com.android.virtualization.terminal.VmLauncherService.Companion.stop
 import com.android.virtualization.terminal.VmLauncherService.VmLauncherServiceCallback
 import com.google.android.material.tabs.TabLayout
 import com.google.android.material.tabs.TabLayoutMediator
@@ -85,7 +82,7 @@
         super.onCreate(savedInstanceState)
         lockOrientationIfNecessary()
 
-        image = getDefault(this)
+        image = InstalledImage.getDefault(this)
 
         val launchInstaller = installIfNecessary()
 
@@ -198,7 +195,7 @@
     override fun dispatchKeyEvent(event: KeyEvent): Boolean {
         if (Build.isDebuggable() && event.keyCode == KeyEvent.KEYCODE_UNKNOWN) {
             if (event.action == KeyEvent.ACTION_UP) {
-                start(this, Exception("Debug: KeyEvent.KEYCODE_UNKNOWN"))
+                ErrorActivity.start(this, Exception("Debug: KeyEvent.KEYCODE_UNKNOWN"))
             }
             return true
         }
@@ -252,7 +249,8 @@
         executorService.shutdown()
         getSystemService<AccessibilityManager>(AccessibilityManager::class.java)
             .removeAccessibilityStateChangeListener(this)
-        stop(this, this)
+        val intent = VmLauncherService.getIntentForShutdown(this, this)
+        startService(intent)
         super.onDestroy()
     }
 
@@ -272,7 +270,7 @@
     override fun onVmError() {
         Log.i(TAG, "onVmError()")
         // TODO: error cause is too simple.
-        start(this, Exception("onVmError"))
+        ErrorActivity.start(this, Exception("onVmError"))
     }
 
     override fun onAccessibilityStateChanged(enabled: Boolean) {
@@ -307,7 +305,7 @@
     }
 
     private fun startVm() {
-        val image = getDefault(this)
+        val image = InstalledImage.getDefault(this)
         if (!image.isInstalled()) {
             return
         }
@@ -322,9 +320,7 @@
         val settingsPendingIntent =
             PendingIntent.getActivity(this, 0, settingsIntent, PendingIntent.FLAG_IMMUTABLE)
 
-        val stopIntent = Intent()
-        stopIntent.setClass(this, VmLauncherService::class.java)
-        stopIntent.setAction(VmLauncherService.ACTION_SHUTDOWN_VM)
+        val stopIntent = VmLauncherService.getIntentForShutdown(this, this)
         val stopPendingIntent =
             PendingIntent.getService(
                 this,
@@ -359,9 +355,20 @@
                 )
                 .build()
 
-        val diskSize = intent.getLongExtra(KEY_DISK_SIZE, image.getSize())
-        run(this, this, notification, getDisplayInfo(), diskSize).onFailure {
-            Log.e(TAG, "Failed to start VM.", it)
+        val diskSize = intent.getLongExtra(EXTRA_DISK_SIZE, image.getSize())
+
+        val intent =
+            VmLauncherService.getIntentForStart(
+                this,
+                this,
+                notification,
+                getDisplayInfo(),
+                diskSize,
+            )
+        try {
+            startForegroundService(intent)
+        } catch (e: ForegroundServiceStartNotAllowedException) {
+            Log.e(TAG, "Failed to start VM", e)
             finish()
         }
     }
@@ -373,7 +380,8 @@
 
     companion object {
         const val TAG: String = "VmTerminalApp"
-        const val KEY_DISK_SIZE: String = "disk_size"
+        const val PREFIX: String = "com.android.virtualization.terminal."
+        const val EXTRA_DISK_SIZE: String = PREFIX + "EXTRA_DISK_SIZE"
         private val TERMINAL_CONNECTION_TIMEOUT_MS: Int
         private const val REQUEST_CODE_INSTALLER = 0x33
         private const val FONT_SIZE_DEFAULT = 13
diff --git a/android/TerminalApp/java/com/android/virtualization/terminal/Runner.kt b/android/TerminalApp/java/com/android/virtualization/terminal/Runner.kt
index 6454cbd..642cb26 100644
--- a/android/TerminalApp/java/com/android/virtualization/terminal/Runner.kt
+++ b/android/TerminalApp/java/com/android/virtualization/terminal/Runner.kt
@@ -27,7 +27,7 @@
 import java.util.concurrent.ForkJoinPool
 
 /** Utility class for creating a VM and waiting for it to finish. */
-internal class Runner private constructor(val vm: VirtualMachine?, callback: Callback) {
+internal class Runner private constructor(val vm: VirtualMachine, callback: Callback) {
     /** Get future about VM's exit status. */
     val exitStatus = callback.finishedSuccessfully
 
diff --git a/android/TerminalApp/java/com/android/virtualization/terminal/SettingsDiskResizeActivity.kt b/android/TerminalApp/java/com/android/virtualization/terminal/SettingsDiskResizeActivity.kt
index 68da45f..af1ae95 100644
--- a/android/TerminalApp/java/com/android/virtualization/terminal/SettingsDiskResizeActivity.kt
+++ b/android/TerminalApp/java/com/android/virtualization/terminal/SettingsDiskResizeActivity.kt
@@ -145,28 +145,30 @@
         // Activity with an extra argument specifying the new size. The actual resizing will be done
         // there.
         // TODO: show progress until the stop is confirmed
-        VmLauncherService.stop(
-            this,
-            object : VmLauncherServiceCallback {
-                override fun onVmStart() {}
+        val intent =
+            VmLauncherService.getIntentForShutdown(
+                this,
+                object : VmLauncherServiceCallback {
+                    override fun onVmStart() {}
 
-                override fun onTerminalAvailable(info: TerminalInfo) {}
+                    override fun onTerminalAvailable(info: TerminalInfo) {}
 
-                override fun onVmStop() {
-                    finish()
+                    override fun onVmStop() {
+                        finish()
 
-                    val intent =
-                        baseContext.packageManager.getLaunchIntentForPackage(
-                            baseContext.packageName
-                        )!!
-                    intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK)
-                    intent.putExtra(MainActivity.KEY_DISK_SIZE, mbToBytes(diskSizeMb))
-                    startActivity(intent)
-                }
+                        val intent =
+                            baseContext.packageManager.getLaunchIntentForPackage(
+                                baseContext.packageName
+                            )!!
+                        intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK)
+                        intent.putExtra(MainActivity.EXTRA_DISK_SIZE, mbToBytes(diskSizeMb))
+                        startActivity(intent)
+                    }
 
-                override fun onVmError() {}
-            },
-        )
+                    override fun onVmError() {}
+                },
+            )
+        startService(intent)
     }
 
     fun updateSliderText(sizeMb: Long) {
diff --git a/android/TerminalApp/java/com/android/virtualization/terminal/VmLauncherService.kt b/android/TerminalApp/java/com/android/virtualization/terminal/VmLauncherService.kt
index 7ec1115..067d540 100644
--- a/android/TerminalApp/java/com/android/virtualization/terminal/VmLauncherService.kt
+++ b/android/TerminalApp/java/com/android/virtualization/terminal/VmLauncherService.kt
@@ -15,7 +15,6 @@
  */
 package com.android.virtualization.terminal
 
-import android.app.ForegroundServiceStartNotAllowedException
 import android.app.Notification
 import android.app.NotificationManager
 import android.app.PendingIntent
@@ -39,9 +38,10 @@
 import android.system.virtualmachine.VirtualMachineException
 import android.util.Log
 import android.widget.Toast
+import androidx.annotation.WorkerThread
 import com.android.system.virtualmachine.flags.Flags
+import com.android.virtualization.terminal.MainActivity.Companion.PREFIX
 import com.android.virtualization.terminal.MainActivity.Companion.TAG
-import com.android.virtualization.terminal.Runner.Companion.create
 import io.grpc.Grpc
 import io.grpc.InsecureServerCredentials
 import io.grpc.Metadata
@@ -63,7 +63,10 @@
 import java.util.concurrent.TimeUnit
 
 class VmLauncherService : Service() {
-    private lateinit var executorService: ExecutorService
+    // Thread pool
+    private lateinit var bgThreads: ExecutorService
+    // Single thread
+    private lateinit var mainWorkerThread: ExecutorService
     private lateinit var image: InstalledImage
 
     // TODO: using lateinit for some fields to avoid null
@@ -89,7 +92,9 @@
 
     override fun onCreate() {
         super.onCreate()
-        executorService = Executors.newCachedThreadPool(TerminalThreadFactory(applicationContext))
+        val threadFactory = TerminalThreadFactory(getApplicationContext())
+        bgThreads = Executors.newCachedThreadPool(threadFactory)
+        mainWorkerThread = Executors.newSingleThreadExecutor(threadFactory)
         image = InstalledImage.getDefault(this)
     }
 
@@ -116,9 +121,15 @@
                 // done.
                 val diskSize = intent.getLongExtra(EXTRA_DISK_SIZE, image.getSize())
 
-                doStart(notification, displayInfo, diskSize, resultReceiver)
+                mainWorkerThread.submit({
+                    doStart(notification, displayInfo, diskSize, resultReceiver)
+                })
+
+                // Do this outside of the main worker thread, so that we don't cause
+                // ForegroundServiceDidNotStartInTimeException
+                startForeground(this.hashCode(), notification)
             }
-            ACTION_SHUTDOWN_VM -> doShutdown(resultReceiver)
+            ACTION_SHUTDOWN_VM -> mainWorkerThread.submit({ doShutdown(resultReceiver) })
             else -> {
                 Log.e(TAG, "Unknown command " + intent.action)
                 stopSelf()
@@ -128,17 +139,13 @@
         return START_NOT_STICKY
     }
 
+    @WorkerThread
     private fun doStart(
         notification: Notification,
         displayInfo: DisplayInfo,
         diskSize: Long,
         resultReceiver: ResultReceiver,
     ) {
-        if (virtualMachine != null) {
-            Log.d(TAG, "VM instance is already started")
-            return
-        }
-
         val image = InstalledImage.getDefault(this)
         val json = ConfigJson.from(this, image.configPath)
         val configBuilder = json.toConfigBuilder(this)
@@ -155,26 +162,24 @@
 
         runner =
             try {
-                create(this, config)
+                Runner.create(this, config)
             } catch (e: VirtualMachineException) {
                 throw RuntimeException("cannot create runner", e)
             }
 
-        virtualMachine = runner!!.vm
-        val mbc = MemBalloonController(this, virtualMachine!!)
+        val virtualMachine = runner!!.vm
+        val mbc = MemBalloonController(this, virtualMachine)
         mbc.start()
 
         runner!!.exitStatus.thenAcceptAsync { success: Boolean ->
             mbc.stop()
-            resultReceiver?.send(if (success) RESULT_STOP else RESULT_ERROR, null)
+            resultReceiver.send(if (success) RESULT_STOP else RESULT_ERROR, null)
             stopSelf()
         }
-        val logDir = getFileStreamPath(virtualMachine!!.name + ".log").toPath()
-        Logger.setup(virtualMachine!!, logDir, executorService)
+        val logDir = getFileStreamPath(virtualMachine.name + ".log").toPath()
+        Logger.setup(virtualMachine, logDir, bgThreads)
 
-        startForeground(this.hashCode(), notification)
-
-        resultReceiver!!.send(RESULT_START, null)
+        resultReceiver.send(RESULT_START, null)
 
         portNotifier = PortNotifier(this)
 
@@ -186,19 +191,19 @@
                     val bundle = Bundle()
                     bundle.putString(KEY_TERMINAL_IPADDRESS, ipAddress)
                     bundle.putInt(KEY_TERMINAL_PORT, port)
-                    resultReceiver!!.send(RESULT_TERMINAL_AVAIL, bundle)
+                    resultReceiver.send(RESULT_TERMINAL_AVAIL, bundle)
                     startDebianServer(ipAddress)
                 },
-                executorService,
+                bgThreads,
             )
             .exceptionallyAsync(
                 { e ->
                     Log.e(TAG, "Failed to start VM", e)
-                    resultReceiver!!.send(RESULT_ERROR, null)
+                    resultReceiver.send(RESULT_ERROR, null)
                     stopSelf()
                     null
                 },
-                executorService,
+                bgThreads,
             )
     }
 
@@ -365,7 +370,7 @@
             return
         }
 
-        executorService.execute(
+        bgThreads.execute(
             Runnable {
                 // TODO(b/373533555): we can use mDNS for that.
                 val debianServicePortFile = File(filesDir, "debian_service_port")
@@ -384,7 +389,9 @@
         }
     }
 
-    private fun doShutdown(resultReceiver: ResultReceiver) {
+    @WorkerThread
+    private fun doShutdown(resultReceiver: ResultReceiver?) {
+        stopForeground(STOP_FOREGROUND_REMOVE)
         if (debianService != null && debianService!!.shutdownDebian()) {
             // During shutdown, change the notification content to indicate that it's closing
             val notification = createNotificationForTerminalClose()
@@ -394,8 +401,10 @@
                 resultReceiver?.send(if (success) RESULT_STOP else RESULT_ERROR, null)
                 stopSelf()
             }
+            runner = null
         } else {
             // If there is no Debian service or it fails to shutdown, just stop the service.
+            runner?.vm?.stop()
             stopSelf()
         }
     }
@@ -407,30 +416,26 @@
     }
 
     override fun onDestroy() {
+        mainWorkerThread.submit({
+            if (runner?.vm?.getStatus() == VirtualMachine.STATUS_RUNNING) {
+                doShutdown(null)
+            }
+        })
         portNotifier?.stop()
         getSystemService<NotificationManager?>(NotificationManager::class.java).cancelAll()
         stopDebianServer()
-        if (virtualMachine != null) {
-            if (virtualMachine!!.getStatus() == VirtualMachine.STATUS_RUNNING) {
-                try {
-                    virtualMachine!!.stop()
-                    stopForeground(STOP_FOREGROUND_REMOVE)
-                } catch (e: VirtualMachineException) {
-                    Log.e(TAG, "failed to stop a VM instance", e)
-                }
-            }
-            virtualMachine = null
-        }
-        executorService.shutdownNow()
+        bgThreads.shutdownNow()
+        mainWorkerThread.shutdown()
         super.onDestroy()
     }
 
     companion object {
-        private const val EXTRA_NOTIFICATION = "EXTRA_NOTIFICATION"
-        private const val ACTION_START_VM: String = "android.virtualization.ACTION_START_VM"
-        const val EXTRA_DISPLAY_INFO = "EXTRA_DISPLAY_INFO"
-        const val EXTRA_DISK_SIZE = "EXTRA_DISK_SIZE"
-        const val ACTION_SHUTDOWN_VM: String = "android.virtualization.ACTION_SHUTDOWN_VM"
+        private const val ACTION_START_VM: String = PREFIX + "ACTION_START_VM"
+        private const val EXTRA_NOTIFICATION = PREFIX + "EXTRA_NOTIFICATION"
+        private const val EXTRA_DISPLAY_INFO = PREFIX + "EXTRA_DISPLAY_INFO"
+        private const val EXTRA_DISK_SIZE = PREFIX + "EXTRA_DISK_SIZE"
+
+        private const val ACTION_SHUTDOWN_VM: String = PREFIX + "ACTION_SHUTDOWN_VM"
 
         private const val RESULT_START = 0
         private const val RESULT_STOP = 1
@@ -482,13 +487,13 @@
             return intent
         }
 
-        fun run(
+        fun getIntentForStart(
             context: Context,
             callback: VmLauncherServiceCallback,
             notification: Notification?,
             displayInfo: DisplayInfo,
             diskSize: Long?,
-        ): Result<Unit> {
+        ): Intent {
             val i = prepareIntent(context, callback)
             i.setAction(ACTION_START_VM)
             i.putExtra(EXTRA_NOTIFICATION, notification)
@@ -496,18 +501,13 @@
             if (diskSize != null) {
                 i.putExtra(EXTRA_DISK_SIZE, diskSize)
             }
-            return try {
-                context.startForegroundService(i)
-                Result.success(Unit)
-            } catch (e: ForegroundServiceStartNotAllowedException) {
-                Result.failure<Unit>(e)
-            }
+            return i
         }
 
-        fun stop(context: Context, callback: VmLauncherServiceCallback) {
+        fun getIntentForShutdown(context: Context, callback: VmLauncherServiceCallback): Intent {
             val i = prepareIntent(context, callback)
             i.setAction(ACTION_SHUTDOWN_VM)
-            context.startService(i)
+            return i
         }
     }
 }