Skip to content

Commit

Permalink
Merge #1278
Browse files Browse the repository at this point in the history
1278: Fix delete crash r=Saviq a=ricab

Fixes #1266. `vm_process` is a pointer now and it is null when in off state. Shutdown is called when deleting, which was dereferencing the ptr to kill the process, even if it wasn't there.

Co-authored-by: Ricardo Abreu <[email protected]>
  • Loading branch information
2 people authored and Saviq committed Jan 7, 2020
1 parent d90f6ef commit 2ca8f1c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/platform/backends/qemu/qemu_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,11 @@ void mp::QemuVirtualMachine::shutdown()
if (state == State::starting)
update_shutdown_status = false;

vm_process->kill();
vm_process->wait_for_finished();
if (vm_process)
{
vm_process->kill();
vm_process->wait_for_finished();
}
}
}

Expand Down Expand Up @@ -390,7 +393,7 @@ void mp::QemuVirtualMachine::on_restart()
void mp::QemuVirtualMachine::ensure_vm_is_running()
{
std::lock_guard<decltype(state_mutex)> lock{state_mutex};
if (!vm_process->running())
if (!vm_process || !vm_process->running())
{
// Have to set 'off' here so there is an actual state change to compare to for
// the cond var's predicate
Expand Down
12 changes: 12 additions & 0 deletions tests/qemu/test_qemu_backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@ TEST_F(QemuBackend, creates_in_off_state)
EXPECT_THAT(machine->current_state(), Eq(mp::VirtualMachine::State::off));
}

TEST_F(QemuBackend, machine_in_off_state_handles_shutdown)
{
mpt::StubVMStatusMonitor stub_monitor;
mp::QemuVirtualMachineFactory backend{data_dir.path()};

auto machine = backend.create_virtual_machine(default_description, stub_monitor);
EXPECT_THAT(machine->current_state(), Eq(mp::VirtualMachine::State::off));

machine->shutdown();
EXPECT_THAT(machine->current_state(), Eq(mp::VirtualMachine::State::off));
}

TEST_F(QemuBackend, machine_start_shutdown_sends_monitoring_events)
{
NiceMock<mpt::MockVMStatusMonitor> mock_monitor;
Expand Down

1 comment on commit 2ca8f1c

@multipass-ci-bot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Snap build available: snap refresh multipass --channel beta/1.0 or multipass_1.0.2_amd64.snap

Please sign in to comment.