[daemon] ensure all outstanding futures are done before exit

include/multipass/async_periodic_download_task.h

@@ -116,6 +116,12 @@ class AsyncPeriodicDownloadTask
         future.waitForFinished();
     }
 
+    void shutdown()
+    {
+        stop_timer();
+        future_watcher.waitForFinished();
+    }
+
 private:
     QTimer timer;
     QFuture<ReturnType> future;

src/daemon/daemon.cpp

@@ -1465,6 +1465,37 @@ mp::Daemon::~Daemon()
     mp::top_catch_all(category, [this] {
         MP_SETTINGS.unregister_handler(instance_mod_handler);
         MP_SETTINGS.unregister_handler(snapshot_mod_handler);
+
+        /**
+         * Wait until all futures are finished, so there will
+         * be no outstanding requests left behind. Otherwise, the
+         * futures might be interrupted halfway and the actions
+         * associated with the futures are not going to be
+         * executed.
+         */
+        for (auto& [_, watcher] : async_future_watchers)
+        {
+            watcher->waitForFinished();
+        }
+
+        /**
+         * AsyncPeriodicDownloadTask maintain its own QFutureWatcher.
+         * Tell it to wrap things up.
+         *
+         * We would like to do that here explicitly instead of in a destructor
+         * since update_manifests_all_task might be using resources from the daemon
+         * and their destruction might be happening before the update_manifests_all_task
+         * object, which makes it troublesome to do this in the AsyncPeriodicDownloadTask
+         * destructor.
+         */
+        update_manifests_all_task.shutdown();
+
+        // waitForFinished() ensures that the futures are finished gracefully
+        // but there's a chance that the signals which are queued during their
+        // execution haven't got executed yet. So, process all the remaining events
+        // in the event loop immediately to ensure that all recipients are notified
+        // before the daemon object destructs.
+        QCoreApplication::processEvents(QEventLoop::AllEvents);
     });
 }
 

tests/test_daemon.cpp

@@ -38,6 +38,7 @@
 #include "mock_vm_blueprint_provider.h"
 #include "mock_vm_image_vault.h"
 #include "path.h"
+#include "signal.h"
 #include "stub_virtual_machine.h"
 #include "tracking_url_downloader.h"
 
@@ -118,6 +119,8 @@ struct Daemon : public mpt::DaemonTestFixture
         EXPECT_CALL(mock_platform, multipass_storage_location()).Times(AnyNumber()).WillRepeatedly(Return(QString()));
         EXPECT_CALL(mock_platform, create_alias_script(_, _)).WillRepeatedly(Return());
         EXPECT_CALL(mock_platform, remove_alias_script(_)).WillRepeatedly(Return());
+        EXPECT_CALL(mock_platform, setup_permission_inheritance(_)).Times(AnyNumber()).WillRepeatedly(Return());
+        EXPECT_CALL(mock_platform, bridge_nomenclature).Times(AnyNumber()).WillRepeatedly(Return("notabridge"));
     }
 
     void SetUp() override
@@ -356,6 +359,52 @@ TEST_F(Daemon, blueprintsURLOverrideIsCorrect)
     EXPECT_EQ(downloader->downloaded_urls.front(), QUrl::fromLocalFile(test_blueprints_zip).toString());
 }
 
+TEST_F(Daemon, ensure_that_on_restart_future_completes)
+{
+    auto mock_factory = use_a_mock_vm_factory();
+
+    constexpr auto instance_json = R"({
+        "yakety-yak": {
+                "deleted": false,
+                "disk_space": "3232323232",
+                "mac_addr": "ab:cd:ef:12:34:56",
+                "mem_size": "2323232323",
+                "metadata": {},
+                "mounts": [],
+                "num_cores": 4,
+                "ssh_username": "ubuntu",
+                "state": 4
+        }
+})";
+    const auto [temp_dir, _] = plant_instance_json(instance_json);
+    config_builder.data_directory = temp_dir->path();
+    config_builder.vault = std::make_unique<NiceMock<mpt::MockVMImageVault>>();
+
+    // This VM was running before, but not now.
+    auto mock_vm = std::make_unique<NiceMock<mpt::MockVirtualMachine>>("yakety-yak");
+    EXPECT_CALL(*mock_vm, current_state)
+        .WillOnce(Return(mp::VirtualMachine::State::stopped))
+        .WillOnce(Return(mp::VirtualMachine::State::stopped));
+    EXPECT_CALL(*mock_vm, start).Times(1);
+
+    mpt::Signal signal;
+    // update_state is called by the finished() handler of the future. If it's called, then
+    // everything's ok.
+    EXPECT_CALL(*mock_vm, update_state).WillOnce([&signal] {
+        // Ensure that update_state is delayed until daemon's destructor call.
+        signal.wait();
+        // Wait a bit to ensure that daemon's destructor has been run
+        std::this_thread::sleep_for(std::chrono::milliseconds{50});
+    });
+    EXPECT_CALL(*mock_vm, wait_until_ssh_up).Times(1);
+    EXPECT_CALL(*mock_factory, create_virtual_machine).WillOnce(Return(std::move(mock_vm)));
+
+    {
+        mp::Daemon daemon{config_builder.build()};
+        signal.signal();
+    }
+}
+
 namespace
 {
 struct DaemonCreateLaunchTestSuite : public Daemon, public WithParamInterface<std::string>
@@ -2378,6 +2427,12 @@ struct DaemonIsBridged : public Daemon,
                          public WithParamInterface<
                              std::tuple<std::vector<mp::NetworkInterfaceInfo>, std::vector<mp::NetworkInterface>, bool>>
 {
+    DaemonIsBridged()
+    {
+        EXPECT_CALL(mock_platform, bridge_nomenclature)
+            .Times(AnyNumber())
+            .WillRepeatedly(Return("this is not a bridge"));
+    }
 };
 
 TEST_P(DaemonIsBridged, is_bridged_works)