Skip to content

Commit

Permalink
[net] Add m_addr_mutex
Browse files Browse the repository at this point in the history
m_next_addr_send and m_next_local_addr_send are currently guarded by
cs_sendProcessing. These fields are only read/writen by the message
handling thread and the annotation was added unnecessarily in commit
b312cd7. See discussion at
bitcoin#13123 (comment).

We add a new m_addr_mutex to guard these fields. Once all the addr relay
fields have been moved to the Peer object in net_processing, they will
be protected by the same mutex.
  • Loading branch information
jnewbery committed Mar 1, 2021
1 parent e52ce9f commit 6a956ba
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 53 deletions.
8 changes: 5 additions & 3 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -542,12 +542,14 @@ class CNode
// Peer selected us as (compact blocks) high-bandwidth peer (BIP152)
std::atomic<bool> m_bip152_highbandwidth_from{false};

// flood relay
/** Protects addr gossip data */
Mutex m_addr_mutex;

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::chrono::microseconds m_next_addr_send GUARDED_BY(m_addr_mutex){0};
std::chrono::microseconds m_next_local_addr_send GUARDED_BY(m_addr_mutex){0};

struct TxRelay {
mutable RecursiveMutex cs_filter;
Expand Down
103 changes: 53 additions & 50 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4417,64 +4417,67 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
// Address refresh broadcast
auto current_time = GetTime<std::chrono::microseconds>();

if (fListen && pto->RelayAddrsWithConn() &&
!::ChainstateActive().IsInitialBlockDownload() &&
pto->m_next_local_addr_send < 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) {
pto->m_addr_known->reset();
}
if (Optional<CAddress> local_addr = GetLocalAddrForPeer(pto)) {
FastRandomContext insecure_rand;
pto->PushAddress(*local_addr, insecure_rand);
{
LOCK(pto->m_addr_mutex);
if (fListen && pto->RelayAddrsWithConn() &&
!::ChainstateActive().IsInitialBlockDownload() &&
pto->m_next_local_addr_send < 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) {
pto->m_addr_known->reset();
}
if (Optional<CAddress> local_addr = GetLocalAddrForPeer(pto)) {
FastRandomContext insecure_rand;
pto->PushAddress(*local_addr, insecure_rand);
}
pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
}
pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
}

//
// Message: addr
//
if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) {
pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL);
std::vector<CAddress> vAddr;
vAddr.reserve(pto->vAddrToSend.size());
assert(pto->m_addr_known);

const char* msg_type;
int make_flags;
if (pto->m_wants_addrv2) {
msg_type = NetMsgType::ADDRV2;
make_flags = ADDRV2_FORMAT;
} else {
msg_type = NetMsgType::ADDR;
make_flags = 0;
}
//
// Message: addr
//
if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) {
pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL);
std::vector<CAddress> vAddr;
vAddr.reserve(pto->vAddrToSend.size());
assert(pto->m_addr_known);

const char* msg_type;
int make_flags;
if (pto->m_wants_addrv2) {
msg_type = NetMsgType::ADDRV2;
make_flags = ADDRV2_FORMAT;
} else {
msg_type = NetMsgType::ADDR;
make_flags = 0;
}

for (const CAddress& addr : pto->vAddrToSend)
{
if (!pto->m_addr_known->contains(addr.GetKey()))
for (const CAddress& addr : pto->vAddrToSend)
{
pto->m_addr_known->insert(addr.GetKey());
vAddr.push_back(addr);
// receiver rejects addr messages larger than MAX_ADDR_TO_SEND
if (vAddr.size() >= MAX_ADDR_TO_SEND)
if (!pto->m_addr_known->contains(addr.GetKey()))
{
m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr));
vAddr.clear();
pto->m_addr_known->insert(addr.GetKey());
vAddr.push_back(addr);
// receiver rejects addr messages larger than MAX_ADDR_TO_SEND
if (vAddr.size() >= MAX_ADDR_TO_SEND)
{
m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr));
vAddr.clear();
}
}
}
pto->vAddrToSend.clear();
if (!vAddr.empty())
m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr));
// we only send the big addr message once
if (pto->vAddrToSend.capacity() > 40)
pto->vAddrToSend.shrink_to_fit();
}
pto->vAddrToSend.clear();
if (!vAddr.empty())
m_connman.PushMessage(pto, msgMaker.Make(make_flags, msg_type, vAddr));
// we only send the big addr message once
if (pto->vAddrToSend.capacity() > 40)
pto->vAddrToSend.shrink_to_fit();
}

// Start block sync
Expand Down

0 comments on commit 6a956ba

Please sign in to comment.