Skip to content

Commit

Permalink
[net] Remove cs_sendProcessing guard from m_next_addr_send and m_next…
Browse files Browse the repository at this point in the history
…_local_addr_send

This locking was mistakenly introduced in PR bitcoin#13123.
Related conversation:
bitcoin#13123 (comment)

Making these fields atomic would ensure safety
if multiple RPC accesses them.
  • Loading branch information
naumenkogs committed Dec 10, 2020
1 parent 38176dc commit 8c9b306
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -995,8 +995,8 @@ class CNode
std::vector<CAddress> vAddrToSend;
std::unique_ptr<CRollingBloomFilter> m_addr_known{nullptr};
bool fGetAddr{false};
std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0};
std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0};
std::atomic<std::chrono::microseconds> m_next_addr_send{std::chrono::microseconds{0}};
std::atomic<std::chrono::microseconds> m_next_local_addr_send{std::chrono::microseconds{0}};

// List of block ids we still have announce.
// There is no final sorting before sending, as they are always sent immediately
Expand Down
6 changes: 3 additions & 3 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4060,14 +4060,14 @@ bool PeerManager::SendMessages(CNode* pto)
// Address refresh broadcast
auto current_time = GetTime<std::chrono::microseconds>();

if (pto->RelayAddrsWithConn() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) {
if (pto->RelayAddrsWithConn() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send.load` < current_time) {
// If we've sent before, clear the bloom filter for the peer, so that our
// self-announcement will actually go out.
// This might be unnecessary if the bloom filter has already rolled
// over since our last self-announcement, but there is only a small
// bandwidth cost that we can incur by doing this (which happens
// once a day on average).
if (pto->m_next_local_addr_send != 0us) {
if (pto->m_next_local_addr_send.load != 0us) {
pto->m_addr_known->reset();
}
AdvertiseLocal(pto);
Expand All @@ -4077,7 +4077,7 @@ bool PeerManager::SendMessages(CNode* pto)
//
// Message: addr
//
if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) {
if (pto->RelayAddrsWithConn() && pto->m_next_addr_send.load() < current_time) {
pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL);
std::vector<CAddress> vAddr;
vAddr.reserve(pto->vAddrToSend.size());
Expand Down

0 comments on commit 8c9b306

Please sign in to comment.