diff --git a/internal/objc/objc.go b/internal/objc/objc.go index 3610cce..cb232ea 100644 --- a/internal/objc/objc.go +++ b/internal/objc/objc.go @@ -28,9 +28,12 @@ void insertNSMutableDictionary(void *dict, char *key, void *val) void releaseNSObject(void* o) { - @autoreleasepool { - [(NSObject*)o release]; - } + [(NSObject*)o release]; +} + +void retainNSObject(void* o) +{ + [(NSObject*)o retain]; } static inline void releaseDispatch(void *queue) @@ -77,11 +80,18 @@ func NewPointer(p unsafe.Pointer) *Pointer { } // release releases allocated resources in objective-c world. +// decrements reference count. func (p *Pointer) release() { C.releaseNSObject(p._ptr) runtime.KeepAlive(p) } +// retain increments reference count in objective-c world. +func (p *Pointer) retain() { + C.retainNSObject(p._ptr) + runtime.KeepAlive(p) +} + // Ptr returns raw pointer. func (o *Pointer) ptr() unsafe.Pointer { if o == nil { @@ -94,6 +104,7 @@ func (o *Pointer) ptr() unsafe.Pointer { type NSObject interface { ptr() unsafe.Pointer release() + retain() } // Release releases allocated resources in objective-c world. @@ -101,6 +112,11 @@ func Release(o NSObject) { o.release() } +// Retain increments reference count in objective-c world. +func Retain(o NSObject) { + o.retain() +} + // Ptr returns unsafe.Pointer of the NSObject func Ptr(o NSObject) unsafe.Pointer { return o.ptr() diff --git a/internal/testhelper/ssh.go b/internal/testhelper/ssh.go new file mode 100644 index 0000000..0ba5bf9 --- /dev/null +++ b/internal/testhelper/ssh.go @@ -0,0 +1,39 @@ +package testhelper + +import ( + "io" + "net" + "testing" + "time" + + "golang.org/x/crypto/ssh" +) + +func NewSshConfig(username, password string) *ssh.ClientConfig { + return &ssh.ClientConfig{ + User: username, + Auth: []ssh.AuthMethod{ssh.Password(password)}, + HostKeyCallback: ssh.InsecureIgnoreHostKey(), + } +} + +func NewSshClient(conn net.Conn, addr string, config *ssh.ClientConfig) (*ssh.Client, error) { + c, chans, reqs, err := ssh.NewClientConn(conn, addr, config) + if err != nil { + return nil, err + } + return ssh.NewClient(c, chans, reqs), nil +} + +func SetKeepAlive(t *testing.T, session *ssh.Session) { + t.Helper() + go func() { + for range time.Tick(5 * time.Second) { + _, err := session.SendRequest("keepalive@codehex.vz", true, nil) + if err != nil && err != io.EOF { + t.Logf("failed to send keep-alive request: %v", err) + return + } + } + }() +} diff --git a/issues_test.go b/issues_test.go index bbf0e63..076cec7 100644 --- a/issues_test.go +++ b/issues_test.go @@ -2,11 +2,14 @@ package vz import ( "errors" + "fmt" "net" "os" "path/filepath" "strings" "testing" + + "github.com/Code-Hex/vz/v3/internal/objc" ) func newTestConfig(t *testing.T) *VirtualMachineConfiguration { @@ -256,3 +259,81 @@ func TestIssue98(t *testing.T) { t.Fatal(err) } } + +func TestIssue119(t *testing.T) { + vmlinuz := "./testdata/Image" + initramfs := "./testdata/initramfs.cpio.gz" + bootLoader, err := NewLinuxBootLoader( + vmlinuz, + WithCommandLine("console=hvc0"), + WithInitrd(initramfs), + ) + if err != nil { + t.Fatal(err) + } + + config, err := setupIssue119Config(bootLoader) + if err != nil { + t.Fatal(err) + } + + vm, err := NewVirtualMachine(config) + if err != nil { + t.Fatal(err) + } + + if canStart := vm.CanStart(); !canStart { + t.Fatal("want CanStart is true") + } + + if err := vm.Start(); err != nil { + t.Fatal(err) + } + + if got := vm.State(); VirtualMachineStateRunning != got { + t.Fatalf("want state %v but got %v", VirtualMachineStateRunning, got) + } + + // Simulates Go's VirtualMachine struct has been destructured but + // Objective-C VZVirtualMachine object has not been destructured. + objc.Retain(vm.pointer) + vm.finalize() + + // sshSession.Run("poweroff") + vm.Pause() +} + +func setupIssue119Config(bootLoader *LinuxBootLoader) (*VirtualMachineConfiguration, error) { + config, err := NewVirtualMachineConfiguration( + bootLoader, + 1, + 512*1024*1024, + ) + if err != nil { + return nil, fmt.Errorf("failed to create a new virtual machine config: %w", err) + } + + // entropy device + entropyConfig, err := NewVirtioEntropyDeviceConfiguration() + if err != nil { + return nil, fmt.Errorf("failed to create entropy device config: %w", err) + } + config.SetEntropyDevicesVirtualMachineConfiguration([]*VirtioEntropyDeviceConfiguration{ + entropyConfig, + }) + + // memory balloon device + memoryBalloonDevice, err := NewVirtioTraditionalMemoryBalloonDeviceConfiguration() + if err != nil { + return nil, fmt.Errorf("failed to create memory balloon device config: %w", err) + } + config.SetMemoryBalloonDevicesVirtualMachineConfiguration([]MemoryBalloonDeviceConfiguration{ + memoryBalloonDevice, + }) + + if _, err := config.Validate(); err != nil { + return nil, err + } + + return config, nil +} diff --git a/virtualization.go b/virtualization.go index 0c69440..a32a318 100644 --- a/virtualization.go +++ b/virtualization.go @@ -9,6 +9,7 @@ package vz */ import "C" import ( + "log" "runtime/cgo" "sync" "unsafe" @@ -74,12 +75,13 @@ type VirtualMachine struct { *pointer dispatchQueue unsafe.Pointer - status cgo.Handle + stateHandle cgo.Handle - mu sync.Mutex + mu *sync.Mutex + finalizeOnce sync.Once } -type machineStatus struct { +type machineState struct { state VirtualMachineState stateNotify chan VirtualMachineState @@ -102,7 +104,7 @@ func NewVirtualMachine(config *VirtualMachineConfiguration) (*VirtualMachine, er cs := (*char)(objc.GetUUID()) dispatchQueue := C.makeDispatchQueue(cs.CString()) - status := cgo.NewHandle(&machineStatus{ + stateHandle := cgo.NewHandle(&machineState{ state: VirtualMachineState(0), stateNotify: make(chan VirtualMachineState), }) @@ -113,21 +115,28 @@ func NewVirtualMachine(config *VirtualMachineConfiguration) (*VirtualMachine, er C.newVZVirtualMachineWithDispatchQueue( objc.Ptr(config), dispatchQueue, - unsafe.Pointer(&status), + unsafe.Pointer(&stateHandle), ), ), dispatchQueue: dispatchQueue, - status: status, + stateHandle: stateHandle, } objc.SetFinalizer(v, func(self *VirtualMachine) { - self.status.Delete() - objc.ReleaseDispatch(self.dispatchQueue) - objc.Release(self) + self.finalize() }) return v, nil } +func (v *VirtualMachine) finalize() { + v.finalizeOnce.Do(func() { + v.stateHandle.Delete() + // deleteStateHandler(unsafe.Pointer(&v.stateHandle)) + objc.ReleaseDispatch(v.dispatchQueue) + objc.Release(v) + }) +} + // SocketDevices return the list of socket devices configured on this virtual machine. // Return an empty array if no socket device is configured. // @@ -147,24 +156,31 @@ func (v *VirtualMachine) SocketDevices() []*VirtioSocketDevice { } //export changeStateOnObserver -func changeStateOnObserver(state C.int, cgoHandlerPtr unsafe.Pointer) { - status := *(*cgo.Handle)(cgoHandlerPtr) +func changeStateOnObserver(newStateRaw C.int, cgoHandlerPtr unsafe.Pointer) { + stateHandler := *(*cgo.Handle)(cgoHandlerPtr) // I expected it will not cause panic. // if caused panic, that's unexpected behavior. - v, _ := status.Value().(*machineStatus) + v, _ := stateHandler.Value().(*machineState) v.mu.Lock() - newState := VirtualMachineState(state) + newState := VirtualMachineState(newStateRaw) + log.Println(newState.String()) v.state = newState // for non-blocking go func() { v.stateNotify <- newState }() 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.status.Value().(*machineStatus) + val, _ := v.stateHandle.Value().(*machineState) val.mu.RLock() defer val.mu.RUnlock() return val.state @@ -174,7 +190,7 @@ func (v *VirtualMachine) State() VirtualMachineState { func (v *VirtualMachine) StateChangedNotify() <-chan VirtualMachineState { // I expected it will not cause panic. // if caused panic, that's unexpected behavior. - val, _ := v.status.Value().(*machineStatus) + val, _ := v.stateHandle.Value().(*machineState) val.mu.RLock() defer val.mu.RUnlock() return val.stateNotify diff --git a/virtualization_test.go b/virtualization_test.go index ebacc02..e63ae83 100644 --- a/virtualization_test.go +++ b/virtualization_test.go @@ -3,14 +3,13 @@ package vz_test import ( "errors" "fmt" - "io" - "net" "os" "syscall" "testing" "time" "github.com/Code-Hex/vz/v3" + "github.com/Code-Hex/vz/v3/internal/testhelper" "golang.org/x/crypto/ssh" ) @@ -107,7 +106,7 @@ func (c *Container) NewSession(t *testing.T) *ssh.Session { if err != nil { t.Fatal(err) } - setKeepAlive(t, sshSession) + testhelper.SetKeepAlive(t, sshSession) return sshSession } @@ -162,11 +161,7 @@ func newVirtualizationMachine( waitState(t, 3*time.Second, vm, vz.VirtualMachineStateStarting) waitState(t, 3*time.Second, vm, vz.VirtualMachineStateRunning) - sshConfig := &ssh.ClientConfig{ - User: "root", - Auth: []ssh.AuthMethod{ssh.Password("passwd")}, - HostKeyCallback: ssh.InsecureIgnoreHostKey(), - } + sshConfig := testhelper.NewSshConfig("root", "passwd") // Workaround for macOS 11 // @@ -192,7 +187,7 @@ RETRY: t.Fatalf("failed to connect vsock: %v", err) } - sshClient, err := newSshClient(conn, ":22", sshConfig) + sshClient, err := testhelper.NewSshClient(conn, ":22", sshConfig) if err != nil { conn.Close() t.Fatalf("failed to create a new ssh client: %v", err) @@ -217,26 +212,6 @@ func waitState(t *testing.T, wait time.Duration, vm *vz.VirtualMachine, want vz. } } -func newSshClient(conn net.Conn, addr string, config *ssh.ClientConfig) (*ssh.Client, error) { - c, chans, reqs, err := ssh.NewClientConn(conn, addr, config) - if err != nil { - return nil, err - } - return ssh.NewClient(c, chans, reqs), nil -} - -func setKeepAlive(t *testing.T, session *ssh.Session) { - go func() { - for range time.Tick(5 * time.Second) { - _, err := session.SendRequest("keepalive@codehex.vz", true, nil) - if err != nil && err != io.EOF { - t.Logf("failed to send keep-alive request: %v", err) - return - } - } - }() -} - func TestRun(t *testing.T) { container := newVirtualizationMachine(t, func(vmc *vz.VirtualMachineConfiguration) error {