Close() shouldn't throw
Close() should handle releasing resources when we are no longer using
a VM (since we implement autoCloseable). It shouldn't throw if th VM
is stopped - that just means nothing is being used.
We could get rid of stop(), or give it the same semantics, but I think
it has uses - if the app should be running then stop() throwing shows
something is wrong.
Add tests. Also minor gratuitous simplification in benchmark code.
Bug: 258397314
Test: atest MicrodroidTests
Change-Id: Ia8a6661ba1664368ba7f09ed0e4dcb8b39469fb8
diff --git a/javalib/api/system-current.txt b/javalib/api/system-current.txt
index 850bffc..16995c5 100644
--- a/javalib/api/system-current.txt
+++ b/javalib/api/system-current.txt
@@ -3,7 +3,7 @@
public class VirtualMachine implements java.lang.AutoCloseable {
method public void clearCallback();
- method public void close() throws android.system.virtualmachine.VirtualMachineException;
+ method public void close();
method @NonNull public android.os.IBinder connectToVsockServer(int) throws android.system.virtualmachine.VirtualMachineException;
method @NonNull public android.os.ParcelFileDescriptor connectVsock(int) throws android.system.virtualmachine.VirtualMachineException;
method public int getCid() throws android.system.virtualmachine.VirtualMachineException;
diff --git a/javalib/src/android/system/virtualmachine/VirtualMachine.java b/javalib/src/android/system/virtualmachine/VirtualMachine.java
index 193d213..dec873f 100644
--- a/javalib/src/android/system/virtualmachine/VirtualMachine.java
+++ b/javalib/src/android/system/virtualmachine/VirtualMachine.java
@@ -65,6 +65,7 @@
import android.system.virtualizationservice.VirtualMachineAppConfig;
import android.system.virtualizationservice.VirtualMachineState;
import android.util.JsonReader;
+import android.util.Log;
import com.android.internal.annotations.GuardedBy;
@@ -105,6 +106,8 @@
*/
@SystemApi
public class VirtualMachine implements AutoCloseable {
+ private static final String TAG = "VirtualMachine";
+
/** Name of the directory under the files directory where all VMs created for the app exist. */
private static final String VM_DIR = "vm";
@@ -754,7 +757,8 @@
* computer; the machine halts immediately. Software running on the virtual machine is not
* notified of the event. A stopped virtual machine can be re-started by calling {@link #run()}.
*
- * @throws VirtualMachineException if the virtual machine could not be stopped.
+ * @throws VirtualMachineException if the virtual machine is not running or could not be
+ * stopped.
* @hide
*/
@SystemApi
@@ -775,15 +779,31 @@
}
/**
- * Stops this virtual machine. See {@link #stop()}.
+ * Stops this virtual machine, if it is running.
*
- * @throws VirtualMachineException if the virtual machine could not be stopped.
+ * @see #stop()
* @hide
*/
@SystemApi
@Override
- public void close() throws VirtualMachineException {
- stop();
+ public void close() {
+ synchronized (mLock) {
+ if (mVirtualMachine == null) {
+ return;
+ }
+ try {
+ if (stateToStatus(mVirtualMachine.getState()) == STATUS_RUNNING) {
+ mVirtualMachine.stop();
+ mVirtualMachine = null;
+ }
+ } catch (RemoteException e) {
+ throw e.rethrowAsRuntimeException();
+ } catch (ServiceSpecificException e) {
+ // Deliberately ignored; this almost certainly means the VM exited just as
+ // we tried to stop it.
+ Log.i(TAG, "Ignoring error on close()", e);
+ }
+ }
}
private static void deleteRecursively(File dir) throws IOException {
diff --git a/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java b/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
index 13e3afd..14a0e39 100644
--- a/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
+++ b/tests/benchmark/src/java/com/android/microdroid/benchmark/MicrodroidBenchmarks.java
@@ -207,8 +207,7 @@
for (int i = 0; i < IO_TEST_TRIAL_COUNT; ++i) {
int port = (mProtectedVm ? 5666 : 6666) + i;
String vmName = "test_vm_io_" + i;
- forceCreateNewVirtualMachine(vmName, config);
- VirtualMachine vm = getVirtualMachineManager().get(vmName);
+ VirtualMachine vm = forceCreateNewVirtualMachine(vmName, config);
BenchmarkVmListener.create(new VsockListener(transferRates, port)).runToFinish(TAG, vm);
}
reportMetrics(transferRates, "vsock/transfer_host_to_vm", "mb_per_sec");
@@ -240,8 +239,7 @@
readRates.clear();
}
String vmName = "test_vm_io_" + i;
- forceCreateNewVirtualMachine(vmName, config);
- VirtualMachine vm = getVirtualMachineManager().get(vmName);
+ VirtualMachine vm = forceCreateNewVirtualMachine(vmName, config);
BenchmarkVmListener.create(new VirtioBlkListener(readRates, isRand))
.runToFinish(TAG, vm);
}
@@ -290,8 +288,7 @@
.setDebugLevel(DEBUG_LEVEL_NONE)
.setMemoryMib(256)
.build();
- forceCreateNewVirtualMachine(vmName, config);
- VirtualMachine vm = getVirtualMachineManager().get(vmName);
+ VirtualMachine vm = forceCreateNewVirtualMachine(vmName, config);
MemoryUsageListener listener = new MemoryUsageListener(this::executeCommand);
BenchmarkVmListener.create(listener).runToFinish(TAG, vm);
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index f6bb7e4..abee36c 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -15,6 +15,9 @@
*/
package com.android.microdroid.test;
+import static android.system.virtualmachine.VirtualMachine.STATUS_DELETED;
+import static android.system.virtualmachine.VirtualMachine.STATUS_RUNNING;
+import static android.system.virtualmachine.VirtualMachine.STATUS_STOPPED;
import static android.system.virtualmachine.VirtualMachineConfig.DEBUG_LEVEL_APP_ONLY;
import static android.system.virtualmachine.VirtualMachineConfig.DEBUG_LEVEL_FULL;
import static android.system.virtualmachine.VirtualMachineConfig.DEBUG_LEVEL_NONE;
@@ -102,11 +105,8 @@
private static final int MIN_MEM_X86_64 = 196;
@Test
- @CddTest(requirements = {
- "9.17/C-1-1",
- "9.17/C-2-1"
- })
- public void connectToVmService() throws Exception {
+ @CddTest(requirements = {"9.17/C-1-1", "9.17/C-2-1"})
+ public void createAndConnectToVm() throws Exception {
assumeSupportedKernel();
VirtualMachineConfig config =
@@ -151,6 +151,36 @@
}
@Test
+ @CddTest(requirements = {"9.17/C-1-1", "9.17/C-2-1"})
+ public void autoCloseVm() throws Exception {
+ assumeSupportedKernel();
+
+ VirtualMachineConfig config =
+ newVmConfigBuilder()
+ .setPayloadBinaryPath("MicrodroidTestNativeLib.so")
+ .setMemoryMib(minMemoryRequired())
+ .build();
+
+ try (VirtualMachine vm = forceCreateNewVirtualMachine("test_vm", config)) {
+ assertThat(vm.getStatus()).isEqualTo(STATUS_STOPPED);
+ // close() implicitly called on stopped VM.
+ }
+
+ try (VirtualMachine vm = getVirtualMachineManager().get("test_vm")) {
+ vm.run();
+ assertThat(vm.getStatus()).isEqualTo(STATUS_RUNNING);
+ // close() implicitly called on running VM.
+ }
+
+ try (VirtualMachine vm = getVirtualMachineManager().get("test_vm")) {
+ assertThat(vm.getStatus()).isEqualTo(STATUS_STOPPED);
+ getVirtualMachineManager().delete("test_vm");
+ assertThat(vm.getStatus()).isEqualTo(STATUS_DELETED);
+ // close() implicitly called on deleted VM.
+ }
+ }
+
+ @Test
@CddTest(requirements = {
"9.17/C-1-1",
"9.17/C-1-2",