Merge pull request #128 from Code-Hex/fix/124-2

fixed issue #124 again

Makefile

@@ -12,6 +12,14 @@ fmt:
 test:
 	go test -exec "go run $(PWD)/cmd/codesign" ./... -timeout 60s -v
 
+.PHONY: test/run
+test/run:
+	go test -exec "go run $(PWD)/cmd/codesign" ./... -timeout 5m -v -run $(TARGET)
+
+.PHONY: test/run/124
+test/run/124:
+	TEST_ISSUE_124=1 $(MAKE) test/run TARGET=TestRunIssue124
+
 .PHONY: download_kernel
 download_kernel:
 	curl --output-dir testdata -LO $(KERNEL_DOWNLOAD_URL)

virtualization.go

@@ -75,7 +75,7 @@ type VirtualMachine struct {
 
 	*pointer
 	dispatchQueue unsafe.Pointer
-	stateHandle   cgo.Handle
+	stateHandle   *machineState
 
 	finalizeOnce sync.Once
 }
@@ -103,18 +103,19 @@ func NewVirtualMachine(config *VirtualMachineConfiguration) (*VirtualMachine, er
 	cs := (*char)(objc.GetUUID())
 	dispatchQueue := C.makeDispatchQueue(cs.CString())
 
-	stateHandle := cgo.NewHandle(&machineState{
+	stateHandle := &machineState{
 		state:       VirtualMachineState(0),
 		stateNotify: infinity.NewChannel[VirtualMachineState](),
-	})
+	}
 
+	stateHandlePtr := cgo.NewHandle(stateHandle)
 	v := &VirtualMachine{
 		id: cs.String(),
 		pointer: objc.NewPointer(
 			C.newVZVirtualMachineWithDispatchQueue(
 				objc.Ptr(config),
 				dispatchQueue,
-				unsafe.Pointer(&stateHandle),
+				unsafe.Pointer(&stateHandlePtr),
 			),
 		),
 		dispatchQueue: dispatchQueue,
@@ -123,6 +124,7 @@ func NewVirtualMachine(config *VirtualMachineConfiguration) (*VirtualMachine, er
 
 	objc.SetFinalizer(v, func(self *VirtualMachine) {
 		self.finalize()
+		stateHandlePtr.Delete()
 	})
 	return v, nil
 }
@@ -165,30 +167,18 @@ func changeStateOnObserver(newStateRaw C.int, cgoHandlerPtr unsafe.Pointer) {
 	v.mu.Unlock()
 }
 
-//export deleteStateHandler
-func deleteStateHandler(cgoHandlerPtr unsafe.Pointer) {
-	stateHandler := *(*cgo.Handle)(cgoHandlerPtr)
-	stateHandler.Delete()
-}
-
 // State represents execution state of the virtual machine.
 func (v *VirtualMachine) State() VirtualMachineState {
-	// I expected it will not cause panic.
-	// if caused panic, that's unexpected behavior.
-	val, _ := v.stateHandle.Value().(*machineState)
-	val.mu.RLock()
-	defer val.mu.RUnlock()
-	return val.state
+	v.stateHandle.mu.RLock()
+	defer v.stateHandle.mu.RUnlock()
+	return v.stateHandle.state
 }
 
 // StateChangedNotify gets notification is changed execution state of the virtual machine.
 func (v *VirtualMachine) StateChangedNotify() <-chan VirtualMachineState {
-	// I expected it will not cause panic.
-	// if caused panic, that's unexpected behavior.
-	val, _ := v.stateHandle.Value().(*machineState)
-	val.mu.RLock()
-	defer val.mu.RUnlock()
-	return val.stateNotify.Out()
+	v.stateHandle.mu.RLock()
+	defer v.stateHandle.mu.RUnlock()
+	return v.stateHandle.stateNotify.Out()
 }
 
 // CanStart returns true if the machine is in a state that can be started.

virtualization_11.h

@@ -13,7 +13,6 @@
 void connectionHandler(void *connection, void *err, void *cgoHandlerPtr);
 void changeStateOnObserver(int state, void *cgoHandler);
 bool shouldAcceptNewConnectionHandler(void *cgoHandler, void *connection, void *socketDevice);
-void deleteStateHandler(void *cgoHandlerPtr);
 
 @interface Observer : NSObject
 - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context;

virtualization_11.m

@@ -39,7 +39,6 @@ - (void)dealloc
     [self removeObserver:_observer forKeyPath:@"state"];
     [_observer release];
     [super dealloc];
-    deleteStateHandler(_stateHandler);
 }
 @end
 

virtualization_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"runtime"
 	"syscall"
 	"testing"
 	"time"
@@ -342,3 +343,71 @@ func TestVirtualMachineStateString(t *testing.T) {
 		}
 	}
 }
+
+func TestRunIssue124(t *testing.T) {
+	if os.Getenv("TEST_ISSUE_124") != "1" {
+		t.Skip()
+	}
+	container := newVirtualizationMachine(t,
+		func(vmc *vz.VirtualMachineConfiguration) error {
+			return setupConsoleConfig(vmc)
+		},
+	)
+	defer container.Close()
+
+	sshSession := container.NewSession(t)
+	defer sshSession.Close()
+
+	vm := container.VirtualMachine
+
+	if got := vm.State(); vz.VirtualMachineStateRunning != got {
+		t.Fatalf("want state %v but got %v", vz.VirtualMachineStateRunning, got)
+	}
+	if got := vm.CanPause(); !got {
+		t.Fatal("want CanPause is true")
+	}
+	if err := vm.Pause(); err != nil {
+		t.Fatal(err)
+	}
+
+	timeout := 5 * time.Second
+	waitState(t, timeout, vm, vz.VirtualMachineStatePausing)
+	waitState(t, timeout, vm, vz.VirtualMachineStatePaused)
+
+	if got := vm.State(); vz.VirtualMachineStatePaused != got {
+		t.Fatalf("want state %v but got %v", vz.VirtualMachineStatePaused, got)
+	}
+	if got := vm.CanResume(); !got {
+		t.Fatal("want CanPause is true")
+	}
+	if err := vm.Resume(); err != nil {
+		t.Fatal(err)
+	}
+
+	waitState(t, timeout, vm, vz.VirtualMachineStateResuming)
+	waitState(t, timeout, vm, vz.VirtualMachineStateRunning)
+
+	if got := vm.CanRequestStop(); !got {
+		t.Fatal("want CanRequestStop is true")
+	}
+
+	ch := make(chan bool)
+	vm.SetMachineStateFinalizer(func() {
+		ch <- true
+	})
+
+	runtime.GC()
+	select {
+	case <-ch:
+		t.Errorf("expected finalizer do not run")
+	case <-time.After(4 * time.Minute):
+	}
+
+	runtime.GC()
+	sshSession.Run("poweroff")
+	waitState(t, timeout, vm, vz.VirtualMachineStateStopped)
+
+	if got := vm.State(); vz.VirtualMachineStateStopped != got {
+		t.Fatalf("want state %v but got %v", vz.VirtualMachineStateStopped, got)
+	}
+}

vz_export_test.go

@@ -0,0 +1,11 @@
+package vz
+
+import (
+	"runtime"
+)
+
+func (v *VirtualMachine) SetMachineStateFinalizer(f func()) {
+	runtime.SetFinalizer(v.stateHandle, func(self *machineState) {
+		f()
+	})
+}