Skip to content

Commit

Permalink
Merge #2417: [net processing] Swap out signals for an interface class
Browse files Browse the repository at this point in the history
21f05c1 net: drop unused connman param (Cory Fields)
50853a2 net: use an interface class rather than signals for message processing (furszy)
67757cd net: pass CConnman via pointer rather than reference (furszy)

Pull request description:

  Another decouple from #2411. Coming from bitcoin#10756.

  > See individual commits.
  > Benefits:
  > * Allows us to begin moving stuff out of CNode and into CNodeState.
  > * Drops boost dependency and overhead
  > * Drops global signal registration
  > * Friendlier backtraces

ACKs for top commit:
  random-zebra:
    ACK 21f05c1
  Fuzzbawls:
    ACK 21f05c1

Tree-SHA512: 8a1ace6d9b8dd2a36d0e1b1465b1f71e7c0a5fcd9a33462210a724cc6eafc119166bd0ee9e9db7862ac41dc62bfa3373c51f575d138b42ec0e140b2524e831d4
  • Loading branch information
furszy committed Jul 1, 2021
2 parents 170beab + 21f05c1 commit f932b2d
Show file tree
Hide file tree
Showing 8 changed files with 154 additions and 179 deletions.
5 changes: 2 additions & 3 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,12 @@ void PrepareShutdown()

// After the threads that potentially access these pointers have been stopped,
// destruct and reset all to nullptr.
peerLogic.reset();
g_connman.reset();
peerLogic.reset();

DumpMasternodes();
DumpBudgets(g_budgetman);
DumpMasternodePayments();
UnregisterNodeSignals(GetNodeSignals());
if (::mempool.IsLoaded() && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
DumpMempool(::mempool);
}
Expand Down Expand Up @@ -1367,7 +1366,6 @@ bool AppInitMain()

peerLogic.reset(new PeerLogicValidation(&connman));
RegisterValidationInterface(peerLogic.get());
RegisterNodeSignals(GetNodeSignals());

// sanitize comments per BIP-0014, format user agent and check total size
std::vector<std::string> uacomments;
Expand Down Expand Up @@ -1958,6 +1956,7 @@ bool AppInitMain()
connOptions.nMaxFeeler = 1;
connOptions.nBestHeight = chainActive.Height();
connOptions.uiInterface = &uiInterface;
connOptions.m_msgproc = peerLogic.get();
connOptions.nSendBufferMaxSize = 1000*gArgs.GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER);
connOptions.nReceiveFloodSize = 1000*gArgs.GetArg("-maxreceivebuffer", DEFAULT_MAXRECEIVEBUFFER);

Expand Down
22 changes: 11 additions & 11 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ std::string strSubVersion;

limitedmap<CInv, int64_t> mapAlreadyAskedFor(MAX_INV_SZ);

// Signals for message handling
static CNodeSignals g_signals;
CNodeSignals& GetNodeSignals() { return g_signals; }

void CConnman::AddOneShot(const std::string& strDest)
{
LOCK(cs_vOneShots);
Expand Down Expand Up @@ -1064,7 +1060,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, "", true);
pnode->AddRef();
pnode->fWhitelisted = whitelisted;
GetNodeSignals().InitializeNode(pnode, *this);
m_msgproc->InitializeNode(pnode);

LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString());

Expand Down Expand Up @@ -1721,7 +1717,7 @@ bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
if (fFeeler)
pnode->fFeeler = true;

GetNodeSignals().InitializeNode(pnode, *this);
m_msgproc->InitializeNode(pnode);
{
LOCK(cs_vNodes);
vNodes.push_back(pnode);
Expand Down Expand Up @@ -1749,16 +1745,17 @@ void CConnman::ThreadMessageHandler()
continue;

// Receive messages
bool fMoreNodeWork = GetNodeSignals().ProcessMessages(pnode, *this, flagInterruptMsgProc);
bool fMoreNodeWork = m_msgproc->ProcessMessages(pnode, flagInterruptMsgProc);
fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend);
if (flagInterruptMsgProc)
return;

// Send messages
{
LOCK(pnode->cs_sendProcessing);
GetNodeSignals().SendMessages(pnode, *this, flagInterruptMsgProc);
m_msgproc->SendMessages(pnode, flagInterruptMsgProc);
}

if (flagInterruptMsgProc)
return;
}
Expand Down Expand Up @@ -1948,6 +1945,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
clientInterface = connOptions.uiInterface;
if (clientInterface)
clientInterface->InitMessage(_("Loading addresses..."));
m_msgproc = connOptions.m_msgproc;
// Load addresses from peers.dat
int64_t nStart = GetTimeMillis();
{
Expand Down Expand Up @@ -1998,12 +1996,13 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();

pnodeLocalHost = new CNode(id, nLocalServices, GetBestHeight(), INVALID_SOCKET, CAddress(CService(local, 0), nLocalServices), 0, nonce);
GetNodeSignals().InitializeNode(pnodeLocalHost, *this);
m_msgproc->InitializeNode(pnodeLocalHost);
}

//
// Start threads
//
assert(m_msgproc);
InterruptSocks5(false);
interruptNet.reset();
flagInterruptMsgProc = false;
Expand Down Expand Up @@ -2124,9 +2123,10 @@ void CConnman::DeleteNode(CNode* pnode)
{
assert(pnode);
bool fUpdateConnectionTime = false;
GetNodeSignals().FinalizeNode(pnode->GetId(), fUpdateConnectionTime);
if(fUpdateConnectionTime)
m_msgproc->FinalizeNode(pnode->GetId(), fUpdateConnectionTime);
if (fUpdateConnectionTime) {
addrman.Connected(pnode->addr);
}
delete pnode;
}

Expand Down
24 changes: 12 additions & 12 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@
#include <arpa/inet.h>
#endif

#include <boost/signals2/signal.hpp>

class CAddrMan;
class CBlockIndex;
class CScheduler;
Expand Down Expand Up @@ -115,7 +113,7 @@ struct CSerializedNetMsg
std::string command;
};


class NetEventsInterface;
class CConnman
{
public:
Expand All @@ -136,6 +134,7 @@ class CConnman
int nMaxFeeler = 0;
int nBestHeight = 0;
CClientUIInterface* uiInterface = nullptr;
NetEventsInterface* m_msgproc = nullptr;
unsigned int nSendBufferMaxSize = 0;
unsigned int nReceiveFloodSize = 0;
};
Expand Down Expand Up @@ -362,6 +361,7 @@ class CConnman
int nMaxFeeler{0};
std::atomic<int> nBestHeight;
CClientUIInterface* clientInterface{nullptr};
NetEventsInterface* m_msgproc{nullptr};

/** SipHasher seeds for deterministic randomness */
const uint64_t nSeed0{0}, nSeed1{0};
Expand Down Expand Up @@ -401,19 +401,19 @@ struct CombinerAll {
}
};

// Signals for message handling
struct CNodeSignals
/**
* Interface for message handling
*/
class NetEventsInterface
{
boost::signals2::signal<bool (CNode*, CConnman&, std::atomic<bool>&), CombinerAll> ProcessMessages;
boost::signals2::signal<bool (CNode*, CConnman&, std::atomic<bool>&), CombinerAll> SendMessages;
boost::signals2::signal<void (CNode*, CConnman&)> InitializeNode;
boost::signals2::signal<void (NodeId, bool&)> FinalizeNode;
public:
virtual bool ProcessMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0;
virtual bool SendMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0;
virtual void InitializeNode(CNode* pnode) = 0;
virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0;
};


CNodeSignals& GetNodeSignals();


enum {
LOCAL_NONE, // unknown
LOCAL_IF, // address a local interface listens on
Expand Down
Loading

0 comments on commit f932b2d

Please sign in to comment.