Jika/oc2r
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rework VMDevice to be more intuitive for typical cases.

Browse Source 
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).
This commit is contained in:
Florian Nücke
2022-01-28 00:55:16 +01:00
parent 82a6702b33
commit ae13fca2e9
15 changed files with 231 additions and 175 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/li/cil/oc2/api/bus/device/provider/BlockDeviceProvider.java
View File

@@ -55,7 +55,7 @@ public interface BlockDeviceProvider extends IForgeRegistryEntry<BlockDeviceProv
/**
* Last-resort cleanup method for devices provided by this provider.
* <p>
* 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.
* <p>

2
src/main/java/li/cil/oc2/api/bus/device/provider/ItemDeviceProvider.java
View File

@@ -65,7 +65,7 @@ public interface ItemDeviceProvider extends IForgeRegistryEntry<ItemDeviceProvid
/**
* Last-resort cleanup method for devices provided by this provider.
* <p>
* 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.
* <p>

38
src/main/java/li/cil/oc2/api/bus/device/rpc/RPCDevice.java
View File

@@ -21,25 +21,25 @@ import java.util.List;
* The lifecycle for {@link RPCDevice}s is as follows:
* <pre>
* ┌──────────────┐ ┌────────────────┐
* │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()├───────────┘
* └─────────┘
* </pre>
*

71
src/main/java/li/cil/oc2/api/bus/device/vm/VMDevice.java
View File

@@ -19,53 +19,30 @@ import li.cil.sedna.api.device.MemoryMappedDevice;
* <p>
* The lifecycle for VMDevices can be depicted as such:
* <pre>
* ┌──────────────────────────────────┐
* │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()├───────────┘
* └─────────┘
* </pre>
* 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();
}

16
src/main/java/li/cil/oc2/common/bus/RPCDeviceBusAdapter.java
View File

@@ -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() {

15
src/main/java/li/cil/oc2/common/bus/device/item/AbstractBlockDeviceVMDevice.java
View File

@@ -81,14 +81,6 @@ public abstract class AbstractBlockDeviceVMDevice<TBlock extends BlockDevice, TI
@Override
public void unmount() {
suspend();
deviceTag = null;
address.clear();
interrupt.clear();
}
@Override
public void suspend() {
closeBlockDevice();
if (blobHandle != null) {
@@ -96,6 +88,13 @@ public abstract class AbstractBlockDeviceVMDevice<TBlock extends BlockDevice, TI
}
}
@Override
public void dispose() {
deviceTag = null;
address.clear();
interrupt.clear();
}
@Override
public void exportToItemStack(final CompoundTag nbt) {
if (blobHandle != null) {

9
src/main/java/li/cil/oc2/common/bus/device/item/AbstractNetworkInterfaceItemDevice.java
View File

@@ -84,15 +84,14 @@ public abstract class AbstractNetworkInterfaceItemDevice extends IdentityProxy<I
@Override
public void unmount() {
suspend();
device = null;
isRunning = false;
address.clear();
interrupt.clear();
}
@Override
public void suspend() {
device = null;
public void dispose() {
address.clear();
interrupt.clear();
}
@Subscribe

13
src/main/java/li/cil/oc2/common/bus/device/item/ByteBufferFlashMemoryVMDevice.java
View File

@@ -67,18 +67,17 @@ public final class ByteBufferFlashMemoryVMDevice extends IdentityProxy<ItemStack
@Override
public void unmount() {
suspend();
deviceTag = null;
address.clear();
}
@Override
public void suspend() {
memoryMap = null;
data = null;
device = null;
}
@Override
public void dispose() {
deviceTag = null;
address.clear();
}
@Subscribe
public void handleInitializingEvent(final VMInitializingEvent event) {
copyDataToMemory(event.programStartAddress());

5
src/main/java/li/cil/oc2/common/bus/device/item/FirmwareFlashMemoryVMDevice.java
View File

@@ -39,12 +39,11 @@ public final class FirmwareFlashMemoryVMDevice extends IdentityProxy<ItemStack>
@Override
public void unmount() {
suspend();
memoryMap = null;
}
@Override
public void suspend() {
memoryMap = null;
public void dispose() {
}
@Subscribe

4
src/main/java/li/cil/oc2/common/bus/device/item/HardDriveVMDeviceWithInitialData.java
View File

@@ -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();

26
src/main/java/li/cil/oc2/common/bus/device/item/MemoryDevice.java
View File

@@ -63,32 +63,30 @@ public final class MemoryDevice extends IdentityProxy<ItemStack> 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

4
src/main/java/li/cil/oc2/common/vm/AbstractVirtualMachine.java
View File

@@ -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;
}

77
src/main/java/li/cil/oc2/common/vm/VMDeviceBusAdapter.java
View File

@@ -12,8 +12,8 @@ import java.util.HashMap;
import java.util.OptionalLong;
public final class VMDeviceBusAdapter {
private final HashMap<VMDevice, ManagedVMContext> deviceContexts = new HashMap<>();
private final ArrayList<VMDevice> incompleteLoads = new ArrayList<>();
private final HashMap<VMDevice, ManagedVMContext> mountedDevices = new HashMap<>();
private final ArrayList<VMDevice> 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<Device> 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());
}
}

11
src/test/java/li/cil/oc2/common/bus/RPCDeviceTests.java
View File

@@ -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);

113
src/test/java/li/cil/oc2/common/bus/VMDeviceTests.java
View File

@@ -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();
});