gui: Watch vm state to terminate when it's stopped #154
Conversation
| @@ -179,6 +198,11 @@ - (instancetype)initWithVirtualMachine:(VZVirtualMachine *)virtualMachine | |||
| self = [super init]; | |||
| _virtualMachine = virtualMachine; | |||
| _virtualMachine.delegate = self; | |||
| _observer = [[VMStateObserver alloc] init]; | |||
There was a problem hiding this comment.
Mmm, when we have chance to release this object...?
| if ([keyPath isEqualToString:@"state"]) { | ||
| int newState = (int)[change[NSKeyValueChangeNewKey] integerValue]; | ||
| if (newState == VZVirtualMachineStateStopped || newState == VZVirtualMachineStateError) { | ||
| [NSApp performSelectorOnMainThread:@selector(terminate:) withObject:context waitUntilDone:NO]; |
There was a problem hiding this comment.
I'm not sure why is not enough code lines at 228 and 234?
There was a problem hiding this comment.
Ah, maybe We don't need state for error...?
There was a problem hiding this comment.
I think this API is better than observing state by self
https://developer.apple.com/documentation/virtualization/vzvirtualmachinedelegate/3656731-virtualmachine?language=objc
There was a problem hiding this comment.
This API is already used in Code-Hex/vz
Lines 207 to 212 in 4f8ae6d
but the log is never printed/the method is never called. The VM did not stop because of an error, maybe it is normal this is not called.
There was a problem hiding this comment.
Could you try this code with Xcode?
NSLog shows only on the Xcode console
I'm sorry my mistake. It uses Stderr
There was a problem hiding this comment.
So... current code is racing with this API and new observing code that you added.
I think better fix:
- Remove code in didStopWithError
- Add code to release observer when dealloc view
And I'm not sure call remove observer in observe handler. (I think this is anti pattern) So it's good calling together with method to release observer
There was a problem hiding this comment.
Thanks for the suggestions, I'll take a look!
There was a problem hiding this comment.
I'm not sure why is not enough code lines at 228 and 234?
(started typing this before your last round of comments, posting it as it adds a bit of explanations)
If we call Stop from the host:
- the guest did not stop the virtual machine, so
guestDidStopVirtualMachineis not called - there was no error, so
didStopWithErroris not called
When calling vm.Stop() while using the GUI, the VM stops, but the GUI is not closed and the application keeps running. Using vm.RequestStop() does not have this issue. This commit modifies example/gui-linux to exhibit the issue.
The GUI code in virtualization_view.m is notified when there was a guest
initiated shutdown ('guestDidStopVirtualMachine') and when there was a
virtualization error ('didStopWithError').
When calling Stop(), the VM is forcefully stopped (similar to pulling
the plug on real hardware). This action is neither a guest initiated
shutdown, nor a virtualization error, so the GUI code does not catch
it. This means after calling vm.Stop(), the GUI main loop will keep
running, and the application using Code-Hex/vz will never exit.
This commit fixes this by adding an observer for VM state changes, and
by calling 'terminate' when the VM state becomes 'stopped' or 'error'.
This fixes Code-Hex#150
cfergeau
force-pushed
the
hardstop-gui
branch
2 times, most recently
from
2cb7af0 to
999f00a
Compare
|
i've only rebased this on top of latest git, I haven't looked into the changes suggested in the review (in short, ignore the latest commits I pushed, nothing new). |
|
This was fixed differently in #173 (comment) |
The GUI code in virtualization_view.m is notified when there was a guest
initiated shutdown ('guestDidStopVirtualMachine') and when there was a
virtualization error ('didStopWithError').
When calling Stop(), the VM is forcefully stopped (similar to pulling the
plug on real hardware). This action is neither a guest initiated shutdown,
nor a virtualization error, so the GUI code does not catch it. This means
after calling vm.Stop(), the GUI main loop will keep running, and the
application using Code-Hex/vz will never exit.
This commit fixes this by adding an observer for VM state changes, and by
calling 'terminate' when the VM state becomes 'stopped' or 'error'.
Which issue(s) this PR fixes:
Fixes #150
Additional documentation
The first commit in this PR allows to reproduce the issue using gui-linux