From ae13fca2e99354f7a33eccee909add4f07d7936d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20N=C3=BCcke?= Date: Fri, 28 Jan 2022 00:55:16 +0100 Subject: [PATCH] Rework VMDevice to be more intuitive for typical cases. unmount() is now called in addition to when suspend() was called before. dispose() is called after unmount() if cleanup is required (machine stopped, device removed). --- .../device/provider/BlockDeviceProvider.java | 2 +- .../device/provider/ItemDeviceProvider.java | 2 +- .../cil/oc2/api/bus/device/rpc/RPCDevice.java | 38 +++--- .../cil/oc2/api/bus/device/vm/VMDevice.java | 71 ++++------- .../oc2/common/bus/RPCDeviceBusAdapter.java | 16 ++- .../item/AbstractBlockDeviceVMDevice.java | 15 ++- .../AbstractNetworkInterfaceItemDevice.java | 9 +- .../item/ByteBufferFlashMemoryVMDevice.java | 13 +- .../item/FirmwareFlashMemoryVMDevice.java | 5 +- .../HardDriveVMDeviceWithInitialData.java | 4 +- .../common/bus/device/item/MemoryDevice.java | 26 ++-- .../oc2/common/vm/AbstractVirtualMachine.java | 4 +- .../cil/oc2/common/vm/VMDeviceBusAdapter.java | 77 +++++------- .../li/cil/oc2/common/bus/RPCDeviceTests.java | 11 ++ .../li/cil/oc2/common/bus/VMDeviceTests.java | 113 ++++++++++++++++-- 15 files changed, 231 insertions(+), 175 deletions(-) diff --git a/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java b/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java index 1d0610fb..634b3edf 100644 --- a/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java +++ b/src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java @@ -55,7 +55,7 @@ public interface BlockDeviceProvider extends IForgeRegistryEntry - * This is the equivalent of {@link RPCDevice#unmount()} or {@link VMDevice#unmount()}, + * This is the equivalent of {@link RPCDevice#dispose()} or {@link VMDevice#dispose()}, * for devices that have gone missing unexpectedly, so this method could no longer be * called on the actual device. *

diff --git a/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java b/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java index 3088a8a0..4a5e1086 100644 --- a/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java +++ b/src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java @@ -65,7 +65,7 @@ public interface ItemDeviceProvider extends IForgeRegistryEntry - * This is the equivalent of {@link RPCDevice#unmount()} or {@link VMDevice#unmount()}, + * This is the equivalent of {@link RPCDevice#dispose()} or {@link VMDevice#dispose()}, * for devices that have gone missing unexpectedly, so this method could no longer be * called on the actual device. *

diff --git a/src/main/java/li/cil/oc2/api/bus/device/rpc/RPCDevice.java b/src/main/java/li/cil/oc2/api/bus/device/rpc/RPCDevice.java index f35f45d4..615c2739 100644 --- a/src/main/java/li/cil/oc2/api/bus/device/rpc/RPCDevice.java +++ b/src/main/java/li/cil/oc2/api/bus/device/rpc/RPCDevice.java @@ -21,25 +21,25 @@ import java.util.List; * The lifecycle for {@link RPCDevice}s is as follows: * 

  * ┌──────────────┐ ┌────────────────┐
- * │serializeNBT()│ │deserializeNBT()◄─────────────────┐
- * └──────────────┘ └───────┬────────┘                 │
- *   May be called          │VM starts or              │
- *   at any time,           │resumes after             │
- *   except while           │load                      │
- *   unloaded...        ┌───▼───┐                      │
- *                      │mount()│                      │
- *                      └───┬───┘                      │
- *                          │VM stops or               │
- *                          │is unloaded               │
- *                          │                          │
- *                     ┌────▼────┐Chunk unloaded       │
- *                     │unmount()├─────────────────────┤
- *                     └────┬────┘                     │
- *                          │VM stopped or             │
- *                          │device removed            │
- *                          │                          │
- *                     ┌────▼────┐                     │
- *                     │dispose()├─────────────────────┘
+ * │serializeNBT()│ │deserializeNBT()◄───────┐
+ * └──────────────┘ └───────┬────────┘       │
+ *   May be called          │VM starts or    │
+ *   at any time,    ┌──────┤resumes after   │
+ *   except while    │      │load            │
+ *   unloaded...     │  ┌───▼───┐            │
+ *                   │  │mount()│            │
+ *                   │  └───┬───┘            │Chunk
+ *                   │      │VM stops or     │unloaded
+ *                   │      │is unloaded     │
+ *                   │      │                │
+ *                   │ ┌────▼────┐           │
+ *                   │ │unmount()├───────────┤
+ *                   │ └────┬────┘           │
+ *                   │      │VM stopped or   │
+ *                   │      │device removed  │
+ *                   │      │                │
+ *                   │ ┌────▼────┐           │
+ *                   └─┤dispose()├───────────┘
  *                     └─────────┘
  *
* diff --git a/src/main/java/li/cil/oc2/api/bus/device/vm/VMDevice.java b/src/main/java/li/cil/oc2/api/bus/device/vm/VMDevice.java index 7de911f6..526924fb 100644 --- a/src/main/java/li/cil/oc2/api/bus/device/vm/VMDevice.java +++ b/src/main/java/li/cil/oc2/api/bus/device/vm/VMDevice.java @@ -19,53 +19,30 @@ import li.cil.sedna.api.device.MemoryMappedDevice; *

* The lifecycle for VMDevices can be depicted as such: * 

- *    ┌──────────────────────────────────┐
- *    │VirtualMachine.isRunning() = false◄────────────────────────┐
- *    └────────────────┬─────────────────┘                        │
- *                     │                                          │
- *          ┌──────────▼───────────┐                              │
- *          │VirtualMachine.start()│                              │
- *          └──────────┬───────────┘                              │
- *                     │                                          │
- *                     │   ┌──────────┐                           │
- *                     │   │Chunk Load│  ┌──────────────────┐     │
- *                     ├───┼──────────◄──┤VMDevice.suspend()◄───┐ │
- *                     │   │World Load│  └──────────────────┘   │ │
- *                     │   └──────────┘                         │ │
- *                     │                                        │ │
- *            ┌────────▼───────┐       ┌────┐                   │ │
- * ┌──────────►VMDevice.mount()◄───────┤Wait◄─────────┐         │ │
- * │          └────────┬───────┘       └──▲─┘         │         │ │
- * │                   │                  │           │         │ │
- * │                   │  ┌───────────────┴─────────┐ │         │ │
- * │                   ├──►VMDeviceLoadResult.fail()│ │         │ │
- * │                   │  └─────────────────────────┘ │         │ │
- * │                   │                              │         │ │
- * │    ┌──────────────▼─────────────┐ ┌──────────────┴───┐     │ │
- * │    │VMDeviceLoadResult.success()│ │VMDevice.unmount()│     │ │
- * │    └──────────────┬─────────────┘ └──────────────▲───┘     │ │
- * │                   │                              │         │ │
- * │                   │        ┌─────────────────────┴───┐     │ │
- * │                   ├────────►     Other VMDevice:     │     │ │
- * │                   │        │VMDeviceLoadResult.fail()│     │ │
- * │                   │        └─────────────────────────┘     │ │
- * │                   │                                        │ │
- * │                   │                     ┌────────────┐     │ │
- * │                   │                     │Chunk Unload│     │ │
- * │                   │                   ┌─►────────────┼─────┘ │
- * │                   │                   │ │World Unload│       │
- * │ ┌─────────────────▼───────────────┐   │ └────────────┘       │
- * │ │VirtualMachine.isRunning() = true├───┤                      │
- * │ └─────┬───────────────────┬───────┘   │ ┌──────────────────┐ │
- * │       │                   │           │ │Computer Shutdown │ │
- * │ ┌─────▼──────┐     ┌──────▼───────┐   └─►──────────────────┤ │
- * └─┤Device Added│     │Device Removed│     │Computer Destroyed│ │
- *   └────────────┘     └──────┬───────┘     └─────────┬────────┘ │
- *                             │                       │          │
- *                    ┌────────▼─────────┐   ┌─────────▼────────┐ │
- *                    │VMDevice.unmount()│   │VMDevice.unmount()├─┘
- *                    └──────────────────┘   └──────────────────┘
+ * ┌──────────────┐ ┌────────────────┐
+ * │serializeNBT()│ │deserializeNBT()◄───────┐
+ * └──────────────┘ └───────┬────────┘       │
+ *   May be called          │VM starts or    │
+ *   at any time,    ┌──────┤resumes after   │
+ *   except while    │      │load            │
+ *   unloaded...     │  ┌───▼───┐            │
+ *                   │  │mount()│            │
+ *                   │  └───┬───┘            │Chunk
+ *                   │      │VM stops or     │unloaded
+ *                   │      │is unloaded     │
+ *                   │      │                │
+ *                   │ ┌────▼────┐           │
+ *                   │ │unmount()├───────────┤
+ *                   │ └────┬────┘           │
+ *                   │      │VM stopped or   │
+ *                   │      │device removed  │
+ *                   │      │                │
+ *                   │ ┌────▼────┐           │
+ *                   └─┤dispose()├───────────┘
+ *                     └─────────┘
  *
+ * Note that if any other {@link VMDevice} fails mounting, all mounted devices + * will immediately unmounted and disposed. * * @see li.cil.oc2.api.bus.device.provider.BlockDeviceProvider * @see li.cil.oc2.api.bus.device.provider.ItemDeviceProvider @@ -106,5 +83,5 @@ public interface VMDevice extends Device { * Intended for soft-releasing resources acquired in {@link #mount(VMContext)}, i.e. non-persisted * unmanaged resources. */ - void suspend(); + void dispose(); } diff --git a/src/main/java/li/cil/oc2/common/bus/RPCDeviceBusAdapter.java b/src/main/java/li/cil/oc2/common/bus/RPCDeviceBusAdapter.java index 2b537f88..d799e7a0 100644 --- a/src/main/java/li/cil/oc2/common/bus/RPCDeviceBusAdapter.java +++ b/src/main/java/li/cil/oc2/common/bus/RPCDeviceBusAdapter.java @@ -74,6 +74,7 @@ public final class RPCDeviceBusAdapter implements Steppable { for (final RPCDevice device : unmountedDevices) { device.mount(); } + mountedDevices.addAll(unmountedDevices); unmountedDevices.clear(); } @@ -82,18 +83,15 @@ public final class RPCDeviceBusAdapter implements Steppable { for (final RPCDevice device : mountedDevices) { device.unmount(); } + + unmountedDevices.addAll(mountedDevices); + mountedDevices.clear(); } public void dispose() { - for (final RPCDevice device : mountedDevices) { - device.unmount(); - device.dispose(); - } - for (final RPCDeviceList device : unmountedDevices) { - device.dispose(); - } - unmountedDevices.addAll(mountedDevices); - mountedDevices.clear(); + unmount(); + + unmountedDevices.forEach(RPCDeviceList::dispose); } public void reset() { diff --git a/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java b/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java index 0eded280..2f44b96c 100644 --- a/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java +++ b/src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java @@ -81,14 +81,6 @@ public abstract class AbstractBlockDeviceVMDevice @Override public void unmount() { - suspend(); + memoryMap = null; } @Override - public void suspend() { - memoryMap = null; + public void dispose() { } @Subscribe diff --git a/src/main/java/li/cil/oc2/common/bus/device/item/HardDriveVMDeviceWithInitialData.java b/src/main/java/li/cil/oc2/common/bus/device/item/HardDriveVMDeviceWithInitialData.java index 84cd5c69..34c8a8d9 100644 --- a/src/main/java/li/cil/oc2/common/bus/device/item/HardDriveVMDeviceWithInitialData.java +++ b/src/main/java/li/cil/oc2/common/bus/device/item/HardDriveVMDeviceWithInitialData.java @@ -67,8 +67,8 @@ public final class HardDriveVMDeviceWithInitialData extends HardDriveVMDevice { @Override protected void closeBlockDevice() { // Join the copy job before releasing the device to avoid writes from thread to closed device. - // Since we use memory mapped memory, closing the device leads to it holding a dead pointer, meaning - // further access to it will hard-crash the JVM. + // Since we use memory mapped memory, closing the device leads to it holding a dead pointer, + // meaning further access to it will hard-crash the JVM. joinCopyJob(); super.closeBlockDevice(); diff --git a/src/main/java/li/cil/oc2/common/bus/device/item/MemoryDevice.java b/src/main/java/li/cil/oc2/common/bus/device/item/MemoryDevice.java index 72658858..9717f8ae 100644 --- a/src/main/java/li/cil/oc2/common/bus/device/item/MemoryDevice.java +++ b/src/main/java/li/cil/oc2/common/bus/device/item/MemoryDevice.java @@ -63,32 +63,30 @@ public final class MemoryDevice extends IdentityProxy implements VMDe @Override public void unmount() { - suspend(); - - // Memory is volatile, so free up our persisted blob when device is unloaded. - if (blobHandle != null) { - BlobStorage.delete(blobHandle); - blobHandle = null; - } - - address.clear(); - } - - @Override - public void suspend() { if (device != null) { try { device.close(); } catch (final Exception e) { LOGGER.error(e); } + + device = null; } if (blobHandle != null) { BlobStorage.close(blobHandle); } + } - device = null; + @Override + public void dispose() { + // Memory is volatile, so free up our persisted blob when device is disposed. + if (blobHandle != null) { + BlobStorage.delete(blobHandle); + blobHandle = null; + } + + address.clear(); } @Override diff --git a/src/main/java/li/cil/oc2/common/vm/AbstractVirtualMachine.java b/src/main/java/li/cil/oc2/common/vm/AbstractVirtualMachine.java index 0e37586c..dc503e61 100644 --- a/src/main/java/li/cil/oc2/common/vm/AbstractVirtualMachine.java +++ b/src/main/java/li/cil/oc2/common/vm/AbstractVirtualMachine.java @@ -90,7 +90,7 @@ public abstract class AbstractVirtualMachine implements VirtualMachine { public void suspend() { joinWorkerThread(); - state.vmAdapter.suspend(); + state.vmAdapter.unmount(); state.rpcAdapter.unmount(); } @@ -204,7 +204,7 @@ public abstract class AbstractVirtualMachine implements VirtualMachine { state.board.reset(); state.rpcAdapter.reset(); state.rpcAdapter.dispose(); - state.vmAdapter.unmount(); + state.vmAdapter.dispose(); runner = null; } diff --git a/src/main/java/li/cil/oc2/common/vm/VMDeviceBusAdapter.java b/src/main/java/li/cil/oc2/common/vm/VMDeviceBusAdapter.java index b4ccb572..ca455e0e 100644 --- a/src/main/java/li/cil/oc2/common/vm/VMDeviceBusAdapter.java +++ b/src/main/java/li/cil/oc2/common/vm/VMDeviceBusAdapter.java @@ -12,8 +12,8 @@ import java.util.HashMap; import java.util.OptionalLong; public final class VMDeviceBusAdapter { - private final HashMap deviceContexts = new HashMap<>(); - private final ArrayList incompleteLoads = new ArrayList<>(); + private final HashMap mountedDevices = new HashMap<>(); + private final ArrayList unmountedDevices = new ArrayList<>(); private BaseAddressProvider baseAddressProvider = unused -> OptionalLong.empty(); /////////////////////////////////////////////////////////////////// @@ -33,28 +33,30 @@ public final class VMDeviceBusAdapter { } public VMDeviceLoadResult mount() { - for (int i = 0; i < incompleteLoads.size(); i++) { - final VMDevice device = incompleteLoads.get(i); + globalContext.joinWorkerThread(); + for (final VMDevice device : unmountedDevices) { final ManagedVMContext context = new ManagedVMContext(globalContext, globalContext, () -> baseAddressProvider.getBaseAddress(device)); - deviceContexts.put(device, context); - final VMDeviceLoadResult result = device.mount(context); context.freeze(); if (!result.wasSuccessful()) { - for (; i >= 0; i--) { - final VMDevice otherDevice = incompleteLoads.get(i); - otherDevice.unmount(); - deviceContexts.get(otherDevice).invalidate(); - } + context.invalidate(); + mountedDevices.forEach((mountedDevice, mountedContext) -> { + mountedDevice.unmount(); + mountedDevice.dispose(); + mountedContext.invalidate(); + }); + mountedDevices.clear(); return result; } + + mountedDevices.put(device, context); } - incompleteLoads.clear(); + unmountedDevices.clear(); globalContext.updateReservations(); @@ -64,21 +66,19 @@ public final class VMDeviceBusAdapter { public void unmount() { globalContext.joinWorkerThread(); - for (final VMDevice device : deviceContexts.keySet()) { + mountedDevices.forEach((device, context) -> { device.unmount(); - } + context.invalidate(); + }); - unload(); + unmountedDevices.addAll(mountedDevices.keySet()); + mountedDevices.clear(); } - public void suspend() { - globalContext.joinWorkerThread(); + public void dispose() { + unmount(); - for (final VMDevice device : deviceContexts.keySet()) { - device.suspend(); - } - - unload(); + unmountedDevices.forEach(VMDevice::dispose); } public void addDevices(final Collection devices) { @@ -86,12 +86,11 @@ public final class VMDeviceBusAdapter { for (final Device device : devices) { if (device instanceof final VMDevice vmDevice) { - final ManagedVMContext context = deviceContexts.put(vmDevice, null); - if (context != null) { - context.invalidate(); + // Add to set of unmounted devices if we don't already track it. It's a set, so + // there won't be duplicates in the unmounted set due to this. + if (!mountedDevices.containsKey(vmDevice)) { + unmountedDevices.add(vmDevice); } - - incompleteLoads.add(vmDevice); } } } @@ -101,28 +100,16 @@ public final class VMDeviceBusAdapter { for (final Device device : devices) { if (device instanceof final VMDevice vmDevice) { - vmDevice.unmount(); - - final ManagedVMContext context = deviceContexts.remove(vmDevice); + final ManagedVMContext context = mountedDevices.remove(vmDevice); if (context != null) { + vmDevice.unmount(); + vmDevice.dispose(); context.invalidate(); + } else { + unmountedDevices.remove(vmDevice); + vmDevice.dispose(); } - - incompleteLoads.remove(vmDevice); } } } - - /////////////////////////////////////////////////////////////////// - - private void unload() { - deviceContexts.forEach((device, context) -> { - if (context != null) { - context.invalidate(); - } - }); - - incompleteLoads.clear(); - incompleteLoads.addAll(deviceContexts.keySet()); - } } diff --git a/src/test/java/li/cil/oc2/common/bus/RPCDeviceTests.java b/src/test/java/li/cil/oc2/common/bus/RPCDeviceTests.java index c3be390d..41dcb916 100644 --- a/src/test/java/li/cil/oc2/common/bus/RPCDeviceTests.java +++ b/src/test/java/li/cil/oc2/common/bus/RPCDeviceTests.java @@ -120,6 +120,17 @@ public final class RPCDeviceTests { verify(device).dispose(); } + @Test + public void devicesHaveMountCalledAfterGlobalUnmount() { + final RPCDevice device = addDevice(); + adapter.resume(controller, true); + adapter.mount(); + adapter.unmount(); + + adapter.mount(); + verify(device, times(2)).mount(); + } + @Test public void deviceListIsStable() { final RPCDevice device1 = mock(RPCDevice.class); diff --git a/src/test/java/li/cil/oc2/common/bus/VMDeviceTests.java b/src/test/java/li/cil/oc2/common/bus/VMDeviceTests.java index ab9b1ebf..b0c01a68 100644 --- a/src/test/java/li/cil/oc2/common/bus/VMDeviceTests.java +++ b/src/test/java/li/cil/oc2/common/bus/VMDeviceTests.java @@ -1,8 +1,8 @@ package li.cil.oc2.common.bus; -import li.cil.oc2.api.bus.device.vm.context.VMContext; import li.cil.oc2.api.bus.device.vm.VMDevice; import li.cil.oc2.api.bus.device.vm.VMDeviceLoadResult; +import li.cil.oc2.api.bus.device.vm.context.VMContext; import li.cil.oc2.common.vm.VMDeviceBusAdapter; import li.cil.oc2.common.vm.context.global.GlobalVMContext; import li.cil.sedna.api.Board; @@ -59,25 +59,72 @@ public final class VMDeviceTests { adapter = new VMDeviceBusAdapter(context); } + @Test + public void addingDeviceDoesNotMountDirectly() { + final VMDevice device = mock(VMDevice.class); + when(device.mount(any())).thenReturn(VMDeviceLoadResult.success()); + + adapter.addDevices(Collections.singleton(device)); + verify(device, never()).mount(any()); + } + @Test public void addedDevicesHaveMountCalled() { + final VMDevice device = mock(VMDevice.class); + when(device.mount(any())).thenReturn(VMDeviceLoadResult.success()); + + adapter.addDevices(Collections.singleton(device)); + assertTrue(adapter.mount().wasSuccessful()); + verify(device).mount(any()); + } + + @Test + public void existingDevicesDoNotHaveMountCalledAgain() { final VMDevice device1 = mock(VMDevice.class); final VMDevice device2 = mock(VMDevice.class); when(device1.mount(any())).thenReturn(VMDeviceLoadResult.success()); when(device2.mount(any())).thenReturn(VMDeviceLoadResult.success()); adapter.addDevices(Collections.singleton(device1)); - assertTrue(adapter.mount().wasSuccessful()); + adapter.mount(); verify(device1).mount(any()); adapter.addDevices(Collections.singleton(device2)); - assertTrue(adapter.mount().wasSuccessful()); + adapter.mount(); verifyNoMoreInteractions(device1); - verify(device2).mount(any()); } @Test - public void removedDevicesHaveUnmountCalled() { + public void deviceFailingMountDoesNotHaveUnmountOrDisposeCalled() { + final VMDevice device = mock(VMDevice.class); + when(device.mount(any())).thenReturn(VMDeviceLoadResult.fail()); + + adapter.addDevices(Collections.singleton(device)); + assertFalse(adapter.mount().wasSuccessful()); + verify(device).mount(any()); + verify(device, never()).unmount(); + verify(device, never()).dispose(); + } + + @Test + public void mountedDevicesAreUnmountedAndDisposedIfOtherMountFails() { + final VMDevice device1 = mock(VMDevice.class); + final VMDevice device2 = mock(VMDevice.class); + when(device1.mount(any())).thenReturn(VMDeviceLoadResult.success()); + when(device2.mount(any())).thenReturn(VMDeviceLoadResult.fail()); + + adapter.addDevices(Collections.singleton(device1)); + adapter.addDevices(Collections.singleton(device2)); + adapter.mount(); + + verify(device1).mount(any()); + verify(device2).mount(any()); + verify(device1).unmount(); + verify(device1).dispose(); + } + + @Test + public void mountedDevicesAreUnmountedAndDisposedWhenRemoved() { final VMDevice device = mock(VMDevice.class); when(device.mount(any())).thenReturn(VMDeviceLoadResult.success()); @@ -86,10 +133,21 @@ public final class VMDeviceTests { adapter.removeDevices(Collections.singleton(device)); verify(device).unmount(); + verify(device).dispose(); } @Test - public void devicesHaveUnloadCalledOnGlobalUnmount() { + public void unmountedDevicesAreDisposedWhenRemoved() { + final VMDevice device = mock(VMDevice.class); + + adapter.addDevices(Collections.singleton(device)); + + adapter.removeDevices(Collections.singleton(device)); + verify(device).dispose(); + } + + @Test + public void mountedDevicesAreUnmountedButNotDisposedOnGlobalUnmount() { final VMDevice device = mock(VMDevice.class); when(device.mount(any())).thenReturn(VMDeviceLoadResult.success()); @@ -98,6 +156,40 @@ public final class VMDeviceTests { adapter.unmount(); verify(device).unmount(); + verify(device, never()).dispose(); + } + + @Test + public void unmountedDevicesAreNotUnmountedAndNotDisposedOnGlobalUnmount() { + final VMDevice device = mock(VMDevice.class); + + adapter.addDevices(Collections.singleton(device)); + + adapter.unmount(); + verify(device, never()).unmount(); + verify(device, never()).dispose(); + } + + @Test + public void mountedDevicesAreUnmountedAndDisposedOnGlobalDispose() { + final VMDevice device = mock(VMDevice.class); + when(device.mount(any())).thenReturn(VMDeviceLoadResult.success()); + adapter.addDevices(Collections.singleton(device)); + adapter.mount(); + + adapter.dispose(); + verify(device).unmount(); + verify(device).dispose(); + } + + @Test + public void unmountedDevicesAreNotUnmountedButDisposedOnGlobalDispose() { + final VMDevice device = mock(VMDevice.class); + adapter.addDevices(Collections.singleton(device)); + + adapter.dispose(); + verify(device, never()).unmount(); + verify(device).dispose(); } @Test @@ -106,11 +198,8 @@ public final class VMDeviceTests { when(device.mount(any())).thenReturn(VMDeviceLoadResult.success()); adapter.addDevices(Collections.singleton(device)); - assertTrue(adapter.mount().wasSuccessful()); - verify(device).mount(any()); - + adapter.mount(); adapter.unmount(); - verify(device).unmount(); assertTrue(adapter.mount().wasSuccessful()); verify(device, times(2)).mount(any()); @@ -188,7 +277,7 @@ public final class VMDeviceTests { final int someInterruptMask = 0x1; assertThrows(IllegalArgumentException.class, () -> - deviceData.context.getInterruptController().raiseInterrupts(someInterruptMask)); + deviceData.context.getInterruptController().raiseInterrupts(someInterruptMask)); } @Test @@ -226,7 +315,7 @@ public final class VMDeviceTests { final VMContext context = invocation.getArgument(0); assertThrows(UnsupportedOperationException.class, () -> - context.getMemoryMap().addDevice(0, mock(MemoryMappedDevice.class))); + context.getMemoryMap().addDevice(0, mock(MemoryMappedDevice.class))); return VMDeviceLoadResult.success(); });