[image-host] Protect manifest with mutex

include/multipass/image_host/base_image_host.h

@@ -23,6 +23,8 @@
 #include <QStringList>
 #include <QTimer>
 
+#include <shared_mutex>
+
 namespace multipass
 {
 
@@ -31,19 +33,29 @@ class BaseVMImageHost : public VMImageHost
 public:
     BaseVMImageHost(URLDownloader* downloader);
 
-    void for_each_entry_do(const Action& action) final;
-    VMImageInfo info_for_full_hash(const std::string& full_hash) final;
+    std::optional<VMImageInfo> info_for(const Query& query) const final;
+    std::vector<std::pair<std::string, VMImageInfo>> all_info_for(const Query& query) const final;
+    VMImageInfo info_for_full_hash(const std::string& full_hash) const final;
+    std::vector<VMImageInfo> all_images_for(const std::string& remote_name,
+                                            const bool allow_unsupported) const final;
+    void for_each_entry_do(const Action& action) const final;
     void update_manifests(const bool force_update);
 
 protected:
     void on_manifest_update_failure(const std::string& details);
     void on_manifest_empty(const std::string& details);
 
-    virtual void for_each_entry_do_impl(const Action& action) = 0;
-    virtual VMImageInfo info_for_full_hash_impl(const std::string& full_hash) = 0;
+    virtual std::optional<VMImageInfo> info_for_impl(const Query& query) const = 0;
+    virtual std::vector<std::pair<std::string, VMImageInfo>> all_info_for_impl(
+        const Query& query) const = 0;
+    virtual VMImageInfo info_for_full_hash_impl(const std::string& full_hash) const = 0;
+    virtual std::vector<VMImageInfo> all_images_for_impl(const std::string& remote_name,
+                                                         const bool allow_unsupported) const = 0;
+    virtual void for_each_entry_do_impl(const Action& action) const = 0;
     virtual void clear() = 0;
     virtual void fetch_manifests(const bool force_update) = 0;
 
+    mutable std::shared_mutex manifest_mutex;
     URLDownloader* const url_downloader;
 };
 

include/multipass/image_host/custom_image_host.h

@@ -42,18 +42,19 @@ class CustomVMImageHost final : public BaseVMImageHost
 public:
     CustomVMImageHost(URLDownloader* downloader);
 
-    std::optional<VMImageInfo> info_for(const Query& query) override;
-    std::vector<std::pair<std::string, VMImageInfo>> all_info_for(const Query& query) override;
-    std::vector<VMImageInfo> all_images_for(const std::string& remote_name,
-                                            const bool allow_unsupported) override;
-    std::vector<std::string> supported_remotes() override;
+    std::vector<std::string> supported_remotes() const override;
 
 private:
-    void for_each_entry_do_impl(const Action& action) override;
-    VMImageInfo info_for_full_hash_impl(const std::string& full_hash) override;
+    std::optional<VMImageInfo> info_for_impl(const Query& query) const override;
+    std::vector<std::pair<std::string, VMImageInfo>> all_info_for_impl(
+        const Query& query) const override;
+    std::vector<VMImageInfo> all_images_for_impl(const std::string& remote_name,
+                                                 const bool allow_unsupported) const override;
+    void for_each_entry_do_impl(const Action& action) const override;
+    VMImageInfo info_for_full_hash_impl(const std::string& full_hash) const override;
     void fetch_manifests(const bool force_update) override;
     void clear() override;
-    CustomManifest* manifest_from(const std::string& remote_name);
+    const CustomManifest& manifest_from(const std::string& remote_name) const;
 
     const QString arch;
     std::pair<std::string, std::unique_ptr<CustomManifest>> manifest;

include/multipass/image_host/ubuntu_image_host.h

@@ -38,18 +38,19 @@ class UbuntuVMImageHost final : public BaseVMImageHost
     UbuntuVMImageHost(std::vector<std::pair<std::string, UbuntuVMImageRemote>> remotes,
                       URLDownloader* downloader);
 
-    std::optional<VMImageInfo> info_for(const Query& query) override;
-    std::vector<std::pair<std::string, VMImageInfo>> all_info_for(const Query& query) override;
-    std::vector<VMImageInfo> all_images_for(const std::string& remote_name,
-                                            const bool allow_unsupported) override;
-    std::vector<std::string> supported_remotes() override;
+    std::vector<std::string> supported_remotes() const override;
 
 private:
-    void for_each_entry_do_impl(const Action& action) override;
-    VMImageInfo info_for_full_hash_impl(const std::string& full_hash) override;
+    std::optional<VMImageInfo> info_for_impl(const Query& query) const override;
+    std::vector<std::pair<std::string, VMImageInfo>> all_info_for_impl(
+        const Query& query) const override;
+    std::vector<VMImageInfo> all_images_for_impl(const std::string& remote_name,
+                                                 const bool allow_unsupported) const override;
+    void for_each_entry_do_impl(const Action& action) const override;
+    VMImageInfo info_for_full_hash_impl(const std::string& full_hash) const override;
     void fetch_manifests(const bool force_update) override;
     void clear() override;
-    SimpleStreamsManifest* manifest_from(const std::string& remote);
+    const SimpleStreamsManifest& manifest_from(const std::string& remote) const;
     const VMImageInfo* match_alias(const QString& key, const SimpleStreamsManifest& manifest) const;
 
     std::vector<std::pair<std::string, std::unique_ptr<SimpleStreamsManifest>>> manifests;

include/multipass/image_host/vm_image_host.h

@@ -36,13 +36,14 @@ class VMImageHost : private DisabledCopyMove
     using Action = std::function<void(const std::string&, const VMImageInfo&)>;
 
     virtual ~VMImageHost() = default;
-    virtual std::optional<VMImageInfo> info_for(const Query& query) = 0;
-    virtual std::vector<std::pair<std::string, VMImageInfo>> all_info_for(const Query& query) = 0;
-    virtual VMImageInfo info_for_full_hash(const std::string& full_hash) = 0;
+    virtual std::optional<VMImageInfo> info_for(const Query& query) const = 0;
+    virtual std::vector<std::pair<std::string, VMImageInfo>> all_info_for(
+        const Query& query) const = 0;
+    virtual VMImageInfo info_for_full_hash(const std::string& full_hash) const = 0;
     virtual std::vector<VMImageInfo> all_images_for(const std::string& remote_name,
-                                                    const bool allow_unsupported) = 0;
-    virtual void for_each_entry_do(const Action& action) = 0;
-    virtual std::vector<std::string> supported_remotes() = 0;
+                                                    const bool allow_unsupported) const = 0;
+    virtual void for_each_entry_do(const Action& action) const = 0;
+    virtual std::vector<std::string> supported_remotes() const = 0;
     virtual void update_manifests(const bool force_update) = 0;
 
 protected:

src/image_host/base_image_host.cpp

@@ -32,18 +32,42 @@ mp::BaseVMImageHost::BaseVMImageHost(URLDownloader* downloader) : url_downloader
 {
 }
 
-void mp::BaseVMImageHost::for_each_entry_do(const Action& action)
+auto mp::BaseVMImageHost::info_for(const Query& query) const -> std::optional<VMImageInfo>
 {
-    for_each_entry_do_impl(action);
+    std::shared_lock lock{manifest_mutex};
+    return info_for_impl(query);
+}
+
+auto mp::BaseVMImageHost::all_info_for(const Query& query) const
+    -> std::vector<std::pair<std::string, VMImageInfo>>
+{
+    std::shared_lock lock{manifest_mutex};
+    return all_info_for_impl(query);
 }
 
-auto mp::BaseVMImageHost::info_for_full_hash(const std::string& full_hash) -> VMImageInfo
+auto mp::BaseVMImageHost::info_for_full_hash(const std::string& full_hash) const -> VMImageInfo
 {
+    std::shared_lock lock{manifest_mutex};
     return info_for_full_hash_impl(full_hash);
 }
 
+auto mp::BaseVMImageHost::all_images_for(const std::string& remote_name,
+                                         const bool allow_unsupported) const
+    -> std::vector<VMImageInfo>
+{
+    std::shared_lock lock{manifest_mutex};
+    return all_images_for_impl(remote_name, allow_unsupported);
+}
+
+void mp::BaseVMImageHost::for_each_entry_do(const Action& action) const
+{
+    std::shared_lock lock{manifest_mutex};
+    for_each_entry_do_impl(action);
+}
+
 void mp::BaseVMImageHost::update_manifests(const bool force_update)
 {
+    std::lock_guard lock{manifest_mutex};
     clear();
     fetch_manifests(force_update);
 }

src/image_host/custom_image_host.cpp

@@ -121,52 +121,50 @@ mp::CustomVMImageHost::CustomVMImageHost(URLDownloader* downloader)
 {
 }
 
-std::optional<mp::VMImageInfo> mp::CustomVMImageHost::info_for(const Query& query)
+std::optional<mp::VMImageInfo> mp::CustomVMImageHost::info_for_impl(const Query& query) const
 {
-    auto custom_manifest = manifest_from(query.remote_name);
+    const auto& custom_manifest = manifest_from(query.remote_name);
 
-    auto it = custom_manifest->image_records.find(query.release);
+    auto it = custom_manifest.image_records.find(query.release);
 
-    if (it == custom_manifest->image_records.end())
+    if (it == custom_manifest.image_records.end())
         return std::nullopt;
 
     return *it->second;
 }
 
-std::vector<std::pair<std::string, mp::VMImageInfo>> mp::CustomVMImageHost::all_info_for(
-    const Query& query)
+std::vector<std::pair<std::string, mp::VMImageInfo>> mp::CustomVMImageHost::all_info_for_impl(
+    const Query& query) const
 {
     std::vector<std::pair<std::string, mp::VMImageInfo>> images;
 
-    if (auto image = info_for(query))
+    if (auto image = info_for_impl(query))
         images.emplace_back(query.remote_name, std::move(*image));
 
     return images;
 }
 
-std::vector<mp::VMImageInfo> mp::CustomVMImageHost::all_images_for(const std::string& remote_name,
-                                                                   const bool allow_unsupported)
+std::vector<mp::VMImageInfo> mp::CustomVMImageHost::all_images_for_impl(
+    const std::string& remote_name,
+    const bool allow_unsupported) const
 {
-    if (auto custom_manifest = manifest_from(remote_name))
-        return custom_manifest->products;
-
-    return {};
+    return manifest_from(remote_name).products;
 }
 
-std::vector<std::string> mp::CustomVMImageHost::supported_remotes()
+std::vector<std::string> mp::CustomVMImageHost::supported_remotes() const
 {
     return {remote};
 }
 
-void mp::CustomVMImageHost::for_each_entry_do_impl(const Action& action)
+void mp::CustomVMImageHost::for_each_entry_do_impl(const Action& action) const
 {
     for (const auto& info : manifest.second->products)
     {
         action(manifest.first, info);
     }
 }
 
-mp::VMImageInfo mp::CustomVMImageHost::info_for_full_hash_impl(const std::string& full_hash)
+mp::VMImageInfo mp::CustomVMImageHost::info_for_full_hash_impl(const std::string& full_hash) const
 {
     for (const auto& product : manifest.second->products)
     {
@@ -198,11 +196,11 @@ void mp::CustomVMImageHost::clear()
     manifest = std::pair<std::string, std::unique_ptr<CustomManifest>>{};
 }
 
-mp::CustomManifest* mp::CustomVMImageHost::manifest_from(const std::string& remote_name)
+const mp::CustomManifest& mp::CustomVMImageHost::manifest_from(const std::string& remote_name) const
 {
-    if (remote_name != manifest.first)
+    if (remote_name != manifest.first || !manifest.second)
         throw std::runtime_error(
             fmt::format("Remote \"{}\" is unknown or unreachable.", remote_name));
 
-    return manifest.second.get();
+    return *manifest.second;
 }

src/image_host/ubuntu_image_host.cpp

@@ -68,9 +68,9 @@ mp::UbuntuVMImageHost::UbuntuVMImageHost(
 {
 }
 
-std::optional<mp::VMImageInfo> mp::UbuntuVMImageHost::info_for(const Query& query)
+std::optional<mp::VMImageInfo> mp::UbuntuVMImageHost::info_for_impl(const Query& query) const
 {
-    auto images = all_info_for(query);
+    auto images = all_info_for_impl(query);
 
     if (images.size() == 0)
         return std::nullopt;
@@ -86,8 +86,8 @@ std::optional<mp::VMImageInfo> mp::UbuntuVMImageHost::info_for(const Query& quer
     return images.front().second;
 }
 
-std::vector<std::pair<std::string, mp::VMImageInfo>> mp::UbuntuVMImageHost::all_info_for(
-    const Query& query)
+std::vector<std::pair<std::string, mp::VMImageInfo>> mp::UbuntuVMImageHost::all_info_for_impl(
+    const Query& query) const
 {
     auto key = key_from(query.release);
 
@@ -106,9 +106,9 @@ std::vector<std::pair<std::string, mp::VMImageInfo>> mp::UbuntuVMImageHost::all_
 
     for (const auto& remote_name : remotes_to_search)
     {
-        auto* manifest = manifest_from(remote_name);
+        const auto& manifest = manifest_from(remote_name);
 
-        if (const auto* info = match_alias(key, *manifest); info)
+        if (const auto* info = match_alias(key, manifest); info)
         {
             if (!info->supported && !query.allow_unsupported)
                 throw mp::UnsupportedImageException(query.release);
@@ -119,7 +119,7 @@ std::vector<std::pair<std::string, mp::VMImageInfo>> mp::UbuntuVMImageHost::all_
         {
             std::unordered_set<std::string> found_hashes;
 
-            for (const auto& entry : manifest->products)
+            for (const auto& entry : manifest.products)
             {
                 if (entry.id.startsWith(key) && (entry.supported || query.allow_unsupported) &&
                     found_hashes.find(entry.id.toStdString()) == found_hashes.end())
@@ -134,7 +134,7 @@ std::vector<std::pair<std::string, mp::VMImageInfo>> mp::UbuntuVMImageHost::all_
     return images;
 }
 
-mp::VMImageInfo mp::UbuntuVMImageHost::info_for_full_hash_impl(const std::string& full_hash)
+mp::VMImageInfo mp::UbuntuVMImageHost::info_for_full_hash_impl(const std::string& full_hash) const
 {
     for (const auto& manifest : manifests)
     {
@@ -150,13 +150,14 @@ mp::VMImageInfo mp::UbuntuVMImageHost::info_for_full_hash_impl(const std::string
     throw mp::ImageNotFoundException(full_hash);
 }
 
-std::vector<mp::VMImageInfo> mp::UbuntuVMImageHost::all_images_for(const std::string& remote_name,
-                                                                   const bool allow_unsupported)
+std::vector<mp::VMImageInfo> mp::UbuntuVMImageHost::all_images_for_impl(
+    const std::string& remote_name,
+    const bool allow_unsupported) const
 {
     std::vector<mp::VMImageInfo> images;
-    auto manifest = manifest_from(remote_name);
+    const auto& manifest = manifest_from(remote_name);
 
-    for (const auto& entry : manifest->products)
+    for (const auto& entry : manifest.products)
     {
         if (entry.supported || allow_unsupported)
         {
@@ -171,7 +172,7 @@ std::vector<mp::VMImageInfo> mp::UbuntuVMImageHost::all_images_for(const std::st
     return images;
 }
 
-void mp::UbuntuVMImageHost::for_each_entry_do_impl(const Action& action)
+void mp::UbuntuVMImageHost::for_each_entry_do_impl(const Action& action) const
 {
     for (const auto& [remote_name, manifest] : manifests)
     {
@@ -182,7 +183,7 @@ void mp::UbuntuVMImageHost::for_each_entry_do_impl(const Action& action)
     }
 }
 
-std::vector<std::string> mp::UbuntuVMImageHost::supported_remotes()
+std::vector<std::string> mp::UbuntuVMImageHost::supported_remotes() const
 {
     std::vector<std::string> supported_remotes;
 
@@ -253,7 +254,8 @@ void mp::UbuntuVMImageHost::clear()
     manifests.clear();
 }
 
-mp::SimpleStreamsManifest* mp::UbuntuVMImageHost::manifest_from(const std::string& remote)
+const mp::SimpleStreamsManifest& mp::UbuntuVMImageHost::manifest_from(
+    const std::string& remote) const
 {
     const auto it = std::find_if(
         manifests.cbegin(),
@@ -267,7 +269,7 @@ mp::SimpleStreamsManifest* mp::UbuntuVMImageHost::manifest_from(const std::strin
                                              "mirror is enabled, please confirm it is valid.",
                                              remote));
 
-    return it->second.get();
+    return *it->second;
 }
 
 const mp::VMImageInfo* mp::UbuntuVMImageHost::match_alias(

tests/unit/mock_image_host.h

@@ -87,18 +87,18 @@ class MockImageHost : public VMImageHost
         ON_CALL(*this, supported_remotes()).WillByDefault(Return(remote));
     };
 
-    MOCK_METHOD(std::optional<VMImageInfo>, info_for, (const Query&), (override));
+    MOCK_METHOD(std::optional<VMImageInfo>, info_for, (const Query&), (const, override));
     MOCK_METHOD((std::vector<std::pair<std::string, VMImageInfo>>),
                 all_info_for,
                 (const Query&),
-                (override));
-    MOCK_METHOD(VMImageInfo, info_for_full_hash, (const std::string&), (override));
+                (const, override));
+    MOCK_METHOD(VMImageInfo, info_for_full_hash, (const std::string&), (const, override));
     MOCK_METHOD(std::vector<VMImageInfo>,
                 all_images_for,
                 (const std::string&, const bool),
-                (override));
-    MOCK_METHOD(void, for_each_entry_do, (const Action&), (override));
-    MOCK_METHOD(std::vector<std::string>, supported_remotes, (), (override));
+                (const, override));
+    MOCK_METHOD(void, for_each_entry_do, (const Action&), (const, override));
+    MOCK_METHOD(std::vector<std::string>, supported_remotes, (), (const, override));
     MOCK_METHOD(void, update_manifests, (bool), (override));
 
     TempFile image;