refactor: store BT peers with std::shared_ptr (#7837)

* refactor: use weak_from_this()

* refactor: store peers as shared_ptr

libtransmission/peer-io.cc

@@ -578,7 +578,7 @@ void tr_peerIo::write_bytes(void const* bytes, size_t n_bytes, bool is_piece_dat
     outbuf_.commit_space(n_bytes);
 
     session_->queue_session_thread(
-        [ptr = std::weak_ptr{ shared_from_this() }]()
+        [ptr = weak_from_this()]()
         {
             if (auto io = ptr.lock(); io)
             {

libtransmission/peer-mgr.cc

@@ -372,7 +372,7 @@ constexpr struct
 class tr_swarm
 {
 public:
-    using Peers = std::vector<tr_peerMsgs*>;
+    using Peers = std::vector<std::shared_ptr<tr_peerMsgs>>;
     using Pool = small::map<tr_socket_address, std::shared_ptr<tr_peer_info>>;
 
     class WishlistMediator final : public Wishlist::Mediator
@@ -482,7 +482,7 @@ public:
         return std::size(peers);
     }
 
-    void remove_peer(tr_peerMsgs* peer)
+    void remove_peer(std::shared_ptr<tr_peerMsgs> const& peer)
     {
         auto const lock = unique_lock();
 
@@ -499,15 +499,13 @@ public:
             peers.erase(iter);
             TR_ASSERT(stats.peer_count == peerCount());
         }
-
-        delete peer;
     }
 
     void remove_all_peers()
     {
         auto tmp = Peers{};
         std::swap(tmp, peers);
-        for (auto* peer : tmp)
+        for (auto const& peer : tmp)
         {
             remove_peer(peer);
         }
@@ -760,9 +758,9 @@ private:
 
     void cancel_all_requests_for_block(tr_block_index_t block, tr_peer const* no_notify)
     {
-        for (auto* peer : peers)
+        for (auto const& peer : peers)
         {
-            maybe_send_cancel_request(peer, block, no_notify);
+            maybe_send_cancel_request(peer.get(), block, no_notify);
         }
     }
 
@@ -781,7 +779,7 @@ private:
 
     void on_torrent_done()
     {
-        std::for_each(std::begin(peers), std::end(peers), [](auto* const peer) { peer->set_interested(false); });
+        std::for_each(std::begin(peers), std::end(peers), [](auto const& peer) { peer->set_interested(false); });
         wishlist.reset();
     }
 
@@ -801,7 +799,7 @@ private:
     {
         bool piece_came_from_peers = false;
 
-        for (auto* const peer : peers)
+        for (auto const& peer : peers)
         {
             // notify the peer that we now have this piece
             peer->on_piece_completed(piece);
@@ -837,9 +835,9 @@ private:
 
         auto const byte_count = tor->piece_size(piece);
 
-        for (auto* const peer : peers)
+        for (auto const& peer : peers)
         {
-            maybe_add_strike(peer);
+            maybe_add_strike(peer.get());
         }
 
         for (auto& webseed : webseeds)
@@ -855,7 +853,7 @@ private:
         // the webseed list may have changed...
         rebuild_webseeds();
 
-        for (auto* peer : peers)
+        for (auto const& peer : peers)
         {
             peer->on_torrent_got_metainfo();
         }
@@ -983,7 +981,7 @@ EXIT:
             auto const it = std::find_if(
                 std::begin(peers),
                 std::end(peers),
-                [&info_that](tr_peerMsgs const* const peer) { return peer->peer_info == info_that; });
+                [&info_that](auto const& peer) { return peer->peer_info == info_that; });
             TR_ASSERT(it != std::end(peers));
             (*it)->disconnect_soon();
 
@@ -1258,7 +1256,7 @@ private:
                 peer_info->set_blocklisted_dirty();
             }
 
-            for (auto* const peer : tor->swarm->peers)
+            for (auto const& peer : tor->swarm->peers)
             {
                 peer->peer_info->set_blocklisted_dirty();
                 if (peer->peer_info->is_blocklisted(blocklists_))
@@ -1345,9 +1343,9 @@ void create_bit_torrent_peer(
 
     tr_swarm* swarm = tor.swarm;
 
-    auto* const
+    auto const
         msgs = tr_peerMsgs::create(tor, std::move(peer_info), std::move(io), peer_id, &tr_swarm::peer_callback_bt, swarm);
-    swarm->peers.push_back(msgs);
+    swarm->peers.emplace_back(msgs);
 
     ++swarm->stats.peer_count;
     ++swarm->stats.peer_from_count[msgs->peer_info->from_first()];
@@ -1657,7 +1655,7 @@ std::vector<tr_pex> tr_peerMgrGetPeers(tr_torrent const* tor, uint8_t address_ty
     {
         auto const& peers = s->peers;
         infos.reserve(std::size(peers));
-        for (auto const* peer : peers)
+        for (auto const& peer : peers)
         {
             if (peer->socket_address().address().type == address_type)
             {
@@ -1737,7 +1735,7 @@ int8_t tr_peerMgrPieceAvailability(tr_torrent const* tor, tr_piece_index_t piece
     }
 
     auto const& peers = tor->swarm->peers;
-    return std::count_if(std::begin(peers), std::end(peers), [piece](auto const* peer) { return peer->has_piece(piece); });
+    return std::count_if(std::begin(peers), std::end(peers), [piece](auto const& peer) { return peer->has_piece(piece); });
 }
 
 void tr_peerMgrTorrentAvailability(tr_torrent const* tor, int8_t* tab, unsigned int n_tabs)
@@ -1794,7 +1792,7 @@ uint64_t tr_peerMgrGetDesiredAvailable(tr_torrent const* tor)
     }
 
     auto available = tr_bitfield{ tor->piece_count() };
-    for (auto const* const peer : swarm->peers)
+    for (auto const& peer : swarm->peers)
     {
         available |= peer->has();
     }
@@ -1952,7 +1950,7 @@ tr_peer_stat* tr_peerMgrPeerStats(tr_torrent const* tor, size_t* setme_count)
         std::begin(peers),
         std::end(peers),
         ret,
-        [&now, &now_msec](auto const* peer) { return peer_stat_helpers::get_peer_stats(peer, now, now_msec); });
+        [&now, &now_msec](auto const& peer) { return peer_stat_helpers::get_peer_stats(peer.get(), now, now_msec); });
 
     *setme_count = n;
     return ret;
@@ -2009,9 +2007,9 @@ void updateInterest(tr_swarm* swarm)
             piece_is_interesting[i] = tor->piece_is_wanted(i) && !tor->has_piece(i);
         }
 
-        for (auto* const peer : peers)
+        for (auto const& peer : peers)
         {
-            peer->set_interested(isPeerInteresting(tor, piece_is_interesting, peer));
+            peer->set_interested(isPeerInteresting(tor, piece_is_interesting, peer.get()));
         }
     }
 }
@@ -2113,7 +2111,7 @@ void rechokeUploads(tr_swarm* s, uint64_t const now)
 
     /* sort the peers by preference and rate */
     auto salter = tr_salt_shaker{};
-    for (auto* const peer : peers)
+    for (auto const& peer : peers)
     {
         if (peer->peer_info->is_upload_only())
         {
@@ -2125,11 +2123,11 @@ void rechokeUploads(tr_swarm* s, uint64_t const now)
             /* choke everyone if we're not uploading */
             peer->set_choke(true);
         }
-        else if (peer != s->optimistic)
+        else if (peer.get() != s->optimistic)
         {
             choked.emplace_back(
-                peer,
-                get_rate(s->tor, peer, now),
+                peer.get(),
+                get_rate(s->tor, peer.get(), now),
                 salter(),
                 peer->peer_is_interested(),
                 peer->peer_is_choked(),
@@ -2289,15 +2287,15 @@ auto constexpr MaxUploadIdleSecs = time_t{ 60 * 5 };
     return false;
 }
 
-void close_peer(tr_peerMsgs* peer)
+void close_peer(std::shared_ptr<tr_peerMsgs> const& peer)
 {
-    TR_ASSERT(peer != nullptr);
+    TR_ASSERT(peer);
     peer->swarm->remove_peer(peer);
 }
 
 constexpr struct
 {
-    [[nodiscard]] static int compare(tr_peerMsgs const* a, tr_peerMsgs const* b) // <=>
+    [[nodiscard]] static int compare(std::shared_ptr<tr_peerMsgs> const& a, std::shared_ptr<tr_peerMsgs> const& b) // <=>
     {
         if (a->is_disconnecting() != b->is_disconnecting())
         {
@@ -2312,18 +2310,19 @@ constexpr struct
         return tr_compare_3way(a->swarm->tor->is_done(), b->swarm->tor->is_done());
     }
 
-    [[nodiscard]] bool operator()(tr_peerMsgs const* a, tr_peerMsgs const* b) const // less than
+    [[nodiscard]] bool operator()(std::shared_ptr<tr_peerMsgs> const& a, std::shared_ptr<tr_peerMsgs> const& b)
+        const // less than
     {
         return compare(a, b) < 0;
     }
 } ComparePeerByMostActive{};
 
-constexpr auto ComparePeerByLeastActive = [](tr_peerMsgs const* a, tr_peerMsgs const* b)
+constexpr auto ComparePeerByLeastActive = [](std::shared_ptr<tr_peerMsgs> const& a, std::shared_ptr<tr_peerMsgs> const& b)
 {
     return ComparePeerByMostActive(b, a);
 };
 
-using bad_peers_t = small::vector<tr_peerMsgs*, 512U>;
+using bad_peers_t = small::vector<std::shared_ptr<tr_peerMsgs>, 512U>;
 
 bad_peers_t& get_peers_to_close(tr_swarm const* const swarm, time_t const now_sec, bad_peers_t& bad_peers_buf)
 {
@@ -2332,9 +2331,9 @@ bad_peers_t& get_peers_to_close(tr_swarm const* const swarm, time_t const now_se
 
     bad_peers_buf.clear();
     bad_peers_buf.reserve(peer_count);
-    for (auto* peer : swarm->peers)
+    for (auto const& peer : swarm->peers)
     {
-        if (shouldPeerBeClosed(swarm, peer, peer_count, now_sec))
+        if (shouldPeerBeClosed(swarm, peer.get(), peer_count, now_sec))
         {
             bad_peers_buf.emplace_back(peer);
         }
@@ -2345,7 +2344,7 @@ bad_peers_t& get_peers_to_close(tr_swarm const* const swarm, time_t const now_se
 
 void close_bad_peers(tr_swarm* s, time_t const now_sec, bad_peers_t& bad_peers_buf)
 {
-    for (auto* peer : get_peers_to_close(s, now_sec, bad_peers_buf))
+    for (auto const& peer : get_peers_to_close(s, now_sec, bad_peers_buf))
     {
         tr_logAddTraceSwarm(peer->swarm, fmt::format("removing bad peer {}", peer->display_name()));
         close_peer(peer);
@@ -2362,7 +2361,7 @@ void enforceSwarmPeerLimit(tr_swarm* swarm, size_t max)
     }
 
     // close all but the `max` most active
-    auto peers = std::vector<tr_peerMsgs*>(n - max);
+    auto peers = std::vector<std::shared_ptr<tr_peerMsgs>>(n - max);
     std::partial_sort_copy(
         std::begin(swarm->peers),
         std::end(swarm->peers),
@@ -2382,7 +2381,7 @@ void enforceSessionPeerLimit(size_t global_peer_limit, tr_torrents& torrents)
     }
 
     // make a list of all the peers
-    auto peers = std::vector<tr_peerMsgs*>{};
+    auto peers = std::vector<std::shared_ptr<tr_peerMsgs>>{};
     peers.reserve(current_size);
     for (auto const* const tor : torrents)
     {
@@ -2535,7 +2534,7 @@ void pumpAllPeers(tr_peerMgr* mgr)
 {
     for (auto* const tor : mgr->torrents_)
     {
-        for (auto* const peer : tor->swarm->peers)
+        for (auto const& peer : tor->swarm->peers)
         {
             peer->pulse();
         }

libtransmission/peer-msgs.cc

@@ -292,7 +292,9 @@ using ReadResult = std::pair<ReadState, size_t /*n_piece_data_bytes_read*/>;
  * @see tr_peer
  * @see tr_peer_info
  */
-class tr_peerMsgsImpl final : public tr_peerMsgs
+class tr_peerMsgsImpl final
+    : public tr_peerMsgs
+    , public std::enable_shared_from_this<tr_peerMsgsImpl>
 {
 public:
     tr_peerMsgsImpl(
@@ -2168,7 +2170,7 @@ tr_peerMsgs::~tr_peerMsgs()
     --n_peers;
 }
 
-tr_peerMsgs* tr_peerMsgs::create(
+std::shared_ptr<tr_peerMsgs> tr_peerMsgs::create(
     tr_torrent& torrent,
     std::shared_ptr<tr_peer_info> peer_info,
     std::shared_ptr<tr_peerIo> io,
@@ -2176,5 +2178,5 @@ tr_peerMsgs* tr_peerMsgs::create(
     tr_peer_callback_bt callback,
     void* callback_data)
 {
-    return new tr_peerMsgsImpl{ torrent, std::move(peer_info), std::move(io), peer_id, callback, callback_data };
+    return std::make_shared<tr_peerMsgsImpl>(torrent, std::move(peer_info), std::move(io), peer_id, callback, callback_data);
 }

libtransmission/peer-msgs.h

@@ -126,7 +126,7 @@ public:
 
     virtual void on_piece_completed(tr_piece_index_t) = 0;
 
-    static tr_peerMsgs* create(
+    static std::shared_ptr<tr_peerMsgs> create(
         tr_torrent& torrent,
         std::shared_ptr<tr_peer_info> peer_info,
         std::shared_ptr<tr_peerIo> io,