Merge pull request #3679 from canonical/fix-start-delete-purge-crash

Fix crash when purge deleting while starting
canonical · Sep 30, 2024 · 602c754 · 602c754
2 parents e1828dd + 3571904
commit 602c754
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 51 deletions.
diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp
@@ -2694,10 +2694,18 @@ void mp::Daemon::on_restart(const std::string& name)
 {
     stop_mounts(name);
     auto future_watcher = create_future_watcher([this, &name]() {
-        auto virtual_machine = operative_instances[name];
-        std::lock_guard<decltype(virtual_machine->state_mutex)> lock{virtual_machine->state_mutex};
-        virtual_machine->state = VirtualMachine::State::running;
-        virtual_machine->update_state();
+        try
+        {
+            auto virtual_machine = operative_instances.at(name);
+
+            std::lock_guard<decltype(virtual_machine->state_mutex)> lock{virtual_machine->state_mutex};
+            virtual_machine->state = VirtualMachine::State::running;
+            virtual_machine->update_state();
+        }
+        catch (const std::out_of_range&)
+        {
+            // logging is dangerous since this thread is probably in a corrupt state
+        }
     });
     future_watcher->setFuture(QtConcurrent::run(&Daemon::async_wait_for_ready_all<StartReply, StartRequest>,
                                                 this,

diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp
@@ -230,53 +230,7 @@ mp::QemuVirtualMachine::QemuVirtualMachine(const VirtualMachineDescription& desc
     convert_to_qcow2_v3_if_necessary(desc.image.image_path,
                                      vm_name); // TODO drop in a couple of releases (went in on v1.13)
 
-    QObject::connect(
-        this, &QemuVirtualMachine::on_delete_memory_snapshot, this,
-        [this] {
-            mpl::log(mpl::Level::debug, vm_name, fmt::format("Deleted memory snapshot"));
-            vm_process->write(hmc_to_qmp_json(QString("delvm ") + suspend_tag));
-            is_starting_from_suspend = false;
-        },
-        Qt::QueuedConnection);
-
-    // The following is the actual code to reset the network via QMP if an IP address is not obtained after
-    // starting from suspend.  This will probably be deprecated in the future.
-    QObject::connect(
-        this, &QemuVirtualMachine::on_reset_network, this,
-        [this] {
-            mpl::log(mpl::Level::debug, vm_name, fmt::format("Resetting the network"));
-
-            auto qmp = QJsonDocument::fromJson(qmp_execute_json("set_link")).object();
-            QJsonObject args;
-            args.insert("name", "virtio-net-pci.0");
-            args.insert("up", false);
-            qmp.insert("arguments", args);
-
-            vm_process->write(QJsonDocument(qmp).toJson());
-
-            args["up"] = true;
-            qmp["arguments"] = args;
-
-            vm_process->write(QJsonDocument(qmp).toJson());
-        },
-        Qt::QueuedConnection);
-
-    QObject::connect(
-        this,
-        &QemuVirtualMachine::on_synchronize_clock,
-        this,
-        [this]() {
-            try
-            {
-                mpl::log(mpl::Level::debug, vm_name, fmt::format("Syncing RTC clock"));
-                ssh_exec("sudo timedatectl set-local-rtc 0 --adjust-system-clock");
-            }
-            catch (const std::exception& e)
-            {
-                mpl::log(mpl::Level::warning, vm_name, fmt::format("Failed to sync clock: {}", e.what()));
-            }
-        },
-        Qt::QueuedConnection);
+    connect_vm_signals();
 }
 
 mp::QemuVirtualMachine::~QemuVirtualMachine()
@@ -323,6 +277,7 @@ void mp::QemuVirtualMachine::start()
     }
 
     vm_process->start();
+    connect_vm_signals();
 
     if (!vm_process->wait_for_started())
     {
@@ -349,6 +304,7 @@ void mp::QemuVirtualMachine::start()
 void mp::QemuVirtualMachine::shutdown(ShutdownPolicy shutdown_policy)
 {
     std::unique_lock<std::mutex> lock{state_mutex};
+    disconnect_vm_signals();
 
     try
     {
@@ -681,6 +637,79 @@ void mp::QemuVirtualMachine::initialize_vm_process()
     });
 }
 
+void mp::QemuVirtualMachine::connect_vm_signals()
+{
+    std::unique_lock lock{vm_signal_mutex};
+
+    if (vm_signals_connected)
+        return;
+
+    QObject::connect(
+        this,
+        &QemuVirtualMachine::on_delete_memory_snapshot,
+        this,
+        [this] {
+            mpl::log(mpl::Level::debug, vm_name, fmt::format("Deleted memory snapshot"));
+            vm_process->write(hmc_to_qmp_json(QString("delvm ") + suspend_tag));
+            is_starting_from_suspend = false;
+        },
+        Qt::QueuedConnection);
+
+    // The following is the actual code to reset the network via QMP if an IP address is not obtained after
+    // starting from suspend.  This will probably be deprecated in the future.
+    QObject::connect(
+        this,
+        &QemuVirtualMachine::on_reset_network,
+        this,
+        [this] {
+            mpl::log(mpl::Level::debug, vm_name, fmt::format("Resetting the network"));
+
+            auto qmp = QJsonDocument::fromJson(qmp_execute_json("set_link")).object();
+            QJsonObject args;
+            args.insert("name", "virtio-net-pci.0");
+            args.insert("up", false);
+            qmp.insert("arguments", args);
+
+            vm_process->write(QJsonDocument(qmp).toJson());
+
+            args["up"] = true;
+            qmp["arguments"] = args;
+
+            vm_process->write(QJsonDocument(qmp).toJson());
+        },
+        Qt::QueuedConnection);
+
+    QObject::connect(
+        this,
+        &QemuVirtualMachine::on_synchronize_clock,
+        this,
+        [this]() {
+            try
+            {
+                mpl::log(mpl::Level::debug, vm_name, fmt::format("Syncing RTC clock"));
+                ssh_exec("sudo timedatectl set-local-rtc 0 --adjust-system-clock");
+            }
+            catch (const std::exception& e)
+            {
+                mpl::log(mpl::Level::warning, vm_name, fmt::format("Failed to sync clock: {}", e.what()));
+            }
+        },
+        Qt::QueuedConnection);
+
+    vm_signals_connected = true;
+}
+
+void mp::QemuVirtualMachine::disconnect_vm_signals()
+{
+    std::unique_lock lock{vm_signal_mutex};
+
+    disconnect(this, &QemuVirtualMachine::on_delete_memory_snapshot, nullptr, nullptr);
+    disconnect(this, &QemuVirtualMachine::on_reset_network, nullptr, nullptr);
+    disconnect(this, &QemuVirtualMachine::on_synchronize_clock, nullptr, nullptr);
+
+    vm_signals_connected = false;
+}
+
 void mp::QemuVirtualMachine::update_cpus(int num_cores)
 {
     assert(num_cores > 0);

diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h
@@ -98,6 +98,9 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine
     void on_restart();
     void initialize_vm_process();
 
+    void connect_vm_signals();
+    void disconnect_vm_signals();
+
     VirtualMachineDescription desc;
     std::unique_ptr<Process> vm_process{nullptr};
     QemuPlatform* qemu_platform;
@@ -107,6 +110,8 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine
     bool update_shutdown_status{true};
     bool is_starting_from_suspend{false};
     bool force_shutdown{false};
+    std::mutex vm_signal_mutex;
+    bool vm_signals_connected{false};
     std::chrono::steady_clock::time_point network_deadline;
 };
 } // namespace multipass