Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gui: Watch vm state to terminate when it's stopped #154

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cfergeau
Copy link
Contributor

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

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
@@ -179,6 +198,11 @@ - (instancetype)initWithVirtualMachine:(VZVirtualMachine *)virtualMachine
self = [super init];
_virtualMachine = virtualMachine;
_virtualMachine.delegate = self;
_observer = [[VMStateObserver alloc] init];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why is not enough code lines at 228 and 234?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, maybe We don't need state for error...?

Copy link
Owner

@Code-Hex Code-Hex Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API is already used in Code-Hex/vz

vz/virtualization_view.m

Lines 207 to 212 in 4f8ae6d

- (void)virtualMachine:(VZVirtualMachine *)virtualMachine didStopWithError:(NSError *)error
{
NSLog(@"VM %@ didStopWithError: %@", virtualMachine, error);
[NSApp performSelectorOnMainThread:@selector(terminate:) withObject:self waitUntilDone:NO];
}

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.

Copy link
Owner

@Code-Hex Code-Hex Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try this code with Xcode?
NSLog shows only on the Xcode console

I'm sorry my mistake. It uses Stderr

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... current code is racing with this API and new observing code that you added.

I think better fix:

  1. Remove code in didStopWithError
  2. 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestions, I'll take a look!

Copy link
Contributor Author

@cfergeau cfergeau Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 guestDidStopVirtualMachine is not called
  • there was no error, so didStopWithError is not called

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

example/gui-linux does not exit when vm.Stop() is used
2 participants