Merge pull request #122 from Code-Hex/fix/119

fixed #119

Author: Code-Hex

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,13 +104,19 @@ func (o *Pointer) ptr() unsafe.Pointer {
 type NSObject interface {
 	ptr() unsafe.Pointer
 	release()
+	retain()
 }
 
 // Release releases allocated resources in objective-c world.
 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()

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("[email protected]", true, nil)
+			if err != nil && err != io.EOF {
+				t.Logf("failed to send keep-alive request: %v", err)
+				return
+			}
+		}
+	}()
+}

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
+}

virtualization.go

@@ -74,12 +74,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 +103,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 +114,26 @@ 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() {
+		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 +153,30 @@ 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)
 	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 +186,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

virtualization_11.h

@@ -13,11 +13,19 @@
 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;
 @end
 
+@interface ObservableVZVirtualMachine : VZVirtualMachine
+- (instancetype)initWithConfiguration:(VZVirtualMachineConfiguration *)configuration
+                                queue:(dispatch_queue_t)queue
+                        statusHandler:(void *)statusHandler;
+- (void)dealloc;
+@end
+
 /* VZVirtioSocketListener */
 @interface VZVirtioSocketListenerDelegateImpl : NSObject <VZVirtioSocketListenerDelegate>
 - (instancetype)initWithHandler:(void *)cgoHandler;

virtualization_11.m

@@ -6,14 +6,6 @@
 
 #import "virtualization_11.h"
 
-char *copyCString(NSString *nss)
-{
-    const char *cc = [nss UTF8String];
-    char *c = calloc([nss length] + 1, 1);
-    strncpy(c, cc, [nss length]);
-    return c;
-}
-
 @implementation Observer
 - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context;
 {
@@ -34,6 +26,32 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
 }
 @end
 
+@implementation ObservableVZVirtualMachine {
+    Observer *_observer;
+    void *_stateHandler;
+};
+- (instancetype)initWithConfiguration:(VZVirtualMachineConfiguration *)configuration
+                                queue:(dispatch_queue_t)queue
+                        statusHandler:(void *)statusHandler
+{
+    self = [super initWithConfiguration:configuration queue:queue];
+    _observer = [[Observer alloc] init];
+    [self addObserver:_observer
+           forKeyPath:@"state"
+              options:NSKeyValueObservingOptionNew
+              context:statusHandler];
+    return self;
+}
+
+- (void)dealloc
+{
+    [self removeObserver:_observer forKeyPath:@"state"];
+    deleteStateHandler(_stateHandler);
+    [_observer release];
+    [super dealloc];
+}
+@end
+
 @implementation VZVirtioSocketListenerDelegateImpl {
     void *_cgoHandler;
 }
@@ -671,16 +689,10 @@ VZVirtioSocketConnectionFlat convertVZVirtioSocketConnection2Flat(void *connecti
 void *newVZVirtualMachineWithDispatchQueue(void *config, void *queue, void *statusHandler)
 {
     if (@available(macOS 11, *)) {
-        VZVirtualMachine *vm = [[VZVirtualMachine alloc]
+        ObservableVZVirtualMachine *vm = [[ObservableVZVirtualMachine alloc]
             initWithConfiguration:(VZVirtualMachineConfiguration *)config
-                            queue:(dispatch_queue_t)queue];
-        @autoreleasepool {
-            Observer *o = [[Observer alloc] init];
-            [vm addObserver:o
-                 forKeyPath:@"state"
-                    options:NSKeyValueObservingOptionNew
-                    context:statusHandler];
-        }
+                            queue:(dispatch_queue_t)queue
+                    statusHandler:statusHandler];
         return vm;
     }