refactor: thread-safe Cache (#6231)

libtransmission/cache.cc

@@ -8,6 +8,7 @@
 #include <cstdint> // uint8_t
 #include <iterator> // std::distance(), std::next(), std::prev()
 #include <memory>
+#include <mutex>
 #include <numeric> // std::accumulate()
 #include <utility> // std::make_pair()
 #include <vector>
@@ -121,14 +122,15 @@ size_t Cache::get_max_blocks(size_t max_bytes) noexcept
 
 int Cache::set_limit(size_t new_limit)
 {
-    max_blocks_ = get_max_blocks(new_limit);
+    auto const lock = std::lock_guard{ mutex_ };
 
+    max_blocks_ = get_max_blocks(new_limit);
     tr_logAddDebug(fmt::format("Maximum cache size set to {} ({} blocks)", tr_formatter_mem_B(new_limit), max_blocks_));
 
     return cache_trim();
 }
 
-Cache::Cache(tr_torrents& torrents, size_t max_bytes)
+Cache::Cache(tr_torrents const& torrents, size_t max_bytes)
     : torrents_{ torrents }
     , max_blocks_(get_max_blocks(max_bytes))
 {
@@ -149,6 +151,8 @@ int Cache::write_block(tr_torrent_id_t tor_id, tr_block_index_t block, std::uniq
         return tr_ioWrite(tor, tor->block_loc(block), std::size(*writeme), std::data(*writeme));
     }
 
+    auto const lock = std::lock_guard{ mutex_ };
+
     auto const key = Key{ tor_id, block };
     auto iter = std::lower_bound(std::begin(blocks_), std::end(blocks_), key, CompareCacheBlockByKey);
     if (iter == std::end(blocks_) || iter->key != key)
@@ -180,23 +184,27 @@ Cache::CIter Cache::get_block(tr_torrent const* torrent, tr_block_info::Location
     return std::end(blocks_);
 }
 
-int Cache::read_block(tr_torrent* torrent, tr_block_info::Location const& loc, uint32_t len, uint8_t* setme)
+int Cache::read_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len, uint8_t* setme)
 {
+    auto lock = std::unique_lock{ mutex_ };
     if (auto const iter = get_block(torrent, loc); iter != std::end(blocks_))
     {
         std::copy_n(std::begin(*iter->buf), len, setme);
         return {};
     }
+    lock.unlock();
 
     return tr_ioRead(torrent, loc, len, setme);
 }
 
-int Cache::prefetch_block(tr_torrent* torrent, tr_block_info::Location const& loc, uint32_t len)
+int Cache::prefetch_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len)
 {
+    auto lock = std::unique_lock{ mutex_ };
     if (auto const iter = get_block(torrent, loc); iter != std::end(blocks_))
     {
         return {}; // already have it
     }
+    lock.unlock();
 
     return tr_ioPrefetch(torrent, loc, len);
 }
@@ -226,6 +234,7 @@ int Cache::flush_file(tr_torrent const* torrent, tr_file_index_t file)
     auto const tor_id = torrent->id();
     auto const [block_begin, block_end] = tr_torGetFileBlockSpan(torrent, file);
 
+    auto const lock = std::lock_guard{ mutex_ };
     return flush_span(
         std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id, block_begin), CompareCacheBlockByKey),
         std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id, block_end), CompareCacheBlockByKey));
@@ -235,6 +244,7 @@ int Cache::flush_torrent(tr_torrent const* torrent)
 {
     auto const tor_id = torrent->id();
 
+    auto const lock = std::lock_guard{ mutex_ };
     return flush_span(
         std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id, 0), CompareCacheBlockByKey),
         std::lower_bound(std::begin(blocks_), std::end(blocks_), std::make_pair(tor_id + 1, 0), CompareCacheBlockByKey));

libtransmission/cache.h

@@ -12,6 +12,7 @@
 #include <cstddef> // for size_t
 #include <cstdint> // for intX_t, uintX_t
 #include <memory> // for std::unique_ptr
+#include <mutex>
 #include <utility> // for std::pair
 #include <vector>
 
@@ -29,15 +30,15 @@ class Cache
 public:
     using BlockData = small::max_size_vector<uint8_t, tr_block_info::BlockSize>;
 
-    Cache(tr_torrents& torrents, size_t max_bytes);
+    Cache(tr_torrents const& torrents, size_t max_bytes);
 
     int set_limit(size_t new_limit);
 
     // @return any error code from cacheTrim()
     int write_block(tr_torrent_id_t tor, tr_block_index_t block, std::unique_ptr<BlockData> writeme);
 
-    int read_block(tr_torrent* torrent, tr_block_info::Location const& loc, uint32_t len, uint8_t* setme);
-    int prefetch_block(tr_torrent* torrent, tr_block_info::Location const& loc, uint32_t len);
+    int read_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len, uint8_t* setme);
+    int prefetch_block(tr_torrent* torrent, tr_block_info::Location const& loc, size_t len);
     int flush_torrent(tr_torrent const* torrent);
     int flush_file(tr_torrent const* torrent, tr_file_index_t file);
 
@@ -75,7 +76,7 @@ private:
 
     [[nodiscard]] CIter get_block(tr_torrent const* torrent, tr_block_info::Location const& loc) noexcept;
 
-    tr_torrents& torrents_;
+    tr_torrents const& torrents_;
 
     Blocks blocks_ = {};
     size_t max_blocks_ = 0;
@@ -85,6 +86,8 @@ private:
     mutable size_t cache_writes_ = 0;
     mutable size_t cache_write_bytes_ = 0;
 
+    mutable std::mutex mutex_;
+
     static constexpr struct
     {
         [[nodiscard]] constexpr bool operator()(Key const& key, CacheBlock const& block)

libtransmission/torrents.cc

@@ -43,19 +43,13 @@ constexpr struct
 
 } // namespace
 
-tr_torrent* tr_torrents::get(std::string_view magnet_link)
+tr_torrent* tr_torrents::get(std::string_view magnet_link) const
 {
     auto magnet = tr_magnet_metainfo{};
     return magnet.parseMagnet(magnet_link) ? get(magnet.info_hash()) : nullptr;
 }
 
-tr_torrent* tr_torrents::get(tr_sha1_digest_t const& hash)
-{
-    auto [begin, end] = std::equal_range(std::begin(by_hash_), std::end(by_hash_), hash, CompareTorrentByHash);
-    return begin == end ? nullptr : *begin;
-}
-
-tr_torrent const* tr_torrents::get(tr_sha1_digest_t const& hash) const
+tr_torrent* tr_torrents::get(tr_sha1_digest_t const& hash) const
 {
     auto [begin, end] = std::equal_range(std::cbegin(by_hash_), std::cend(by_hash_), hash, CompareTorrentByHash);
     return begin == end ? nullptr : *begin;

libtransmission/torrents.h

@@ -35,22 +35,16 @@ public:
     void remove(tr_torrent const* tor, time_t current_time);
 
     // O(1)
-    [[nodiscard]] TR_CONSTEXPR20 tr_torrent* get(tr_torrent_id_t id)
+    [[nodiscard]] TR_CONSTEXPR20 tr_torrent* get(tr_torrent_id_t id) const
     {
         auto const uid = static_cast<size_t>(id);
         return uid >= std::size(by_id_) ? nullptr : by_id_.at(uid);
     }
 
     // O(log n)
-    [[nodiscard]] tr_torrent const* get(tr_sha1_digest_t const& hash) const;
-    [[nodiscard]] tr_torrent* get(tr_sha1_digest_t const& hash);
+    [[nodiscard]] tr_torrent* get(tr_sha1_digest_t const& hash) const;
 
-    [[nodiscard]] tr_torrent const* get(tr_torrent_metainfo const& metainfo) const
-    {
-        return get(metainfo.info_hash());
-    }
-
-    [[nodiscard]] tr_torrent* get(tr_torrent_metainfo const& metainfo)
+    [[nodiscard]] tr_torrent* get(tr_torrent_metainfo const& metainfo) const
     {
         return get(metainfo.info_hash());
     }
@@ -61,7 +55,7 @@ public:
     // These convenience functions use get(tr_sha1_digest_t const&)
     // after parsing the magnet link to get the info hash. If you have
     // the info hash already, use get() instead to avoid excess parsing.
-    [[nodiscard]] tr_torrent* get(std::string_view magnet_link);
+    [[nodiscard]] tr_torrent* get(std::string_view magnet_link) const;
 
     template<typename T>
     [[nodiscard]] bool contains(T const& key) const