diff --git a/Makefile b/Makefile index 4bd73a3..bef595d 100644 --- a/Makefile +++ b/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) diff --git a/virtualization.go b/virtualization.go index 0cb9d61..00c79cb 100644 --- a/virtualization.go +++ b/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. diff --git a/virtualization_11.h b/virtualization_11.h index d24ad71..c92f827 100644 --- a/virtualization_11.h +++ b/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; diff --git a/virtualization_11.m b/virtualization_11.m index 2df1f21..af7785e 100644 --- a/virtualization_11.m +++ b/virtualization_11.m @@ -39,7 +39,6 @@ - (void)dealloc [self removeObserver:_observer forKeyPath:@"state"]; [_observer release]; [super dealloc]; - deleteStateHandler(_stateHandler); } @end diff --git a/virtualization_test.go b/virtualization_test.go index e63ae83..f42d8dd 100644 --- a/virtualization_test.go +++ b/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) + } +} diff --git a/vz_export_test.go b/vz_export_test.go new file mode 100644 index 0000000..7c60def --- /dev/null +++ b/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() + }) +}