Skip to content

Commit

Permalink
Merge bitcoin#10756: net processing: swap out signals for an interfac…
Browse files Browse the repository at this point in the history
…e class

2525b97 net: stop both net/net_processing before destroying them (Cory Fields)
80e2e9d net: drop unused connman param (Cory Fields)
8ad663c net: use an interface class rather than signals for message processing (Cory Fields)
28f11e9 net: pass CConnman via pointer rather than reference (Cory Fields)

Pull request description:

  See individual commits.
  Benefits:
  - Allows us to begin moving stuff out of CNode and into CNodeState (after bitcoin#10652 and follow-ups)
  - Drops boost dependency and overhead
  - Drops global signal registration
  - Friendlier backtraces

Tree-SHA512: af2038c959dbec25f0c90c74c88dc6a630e6b9e984adf52aceadd6954aa463b6aadfccf979c2459a9f3354326b5077ee02048128eda2a649236fadb595b66ee3
  • Loading branch information
laanwj authored and codablock committed Sep 25, 2019
1 parent 1cf3a1e commit 022e80a
Show file tree
Hide file tree
Showing 8 changed files with 180 additions and 201 deletions.
9 changes: 5 additions & 4 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,14 @@ void PrepareShutdown()
}
#endif
MapPort(false);

// Because these depend on each-other, we make sure that neither can be
// using the other before destroying them.
UnregisterValidationInterface(peerLogic.get());
peerLogic.reset();
if (g_connman) {
// make sure to stop all threads before g_connman is reset to nullptr as these threads might still be accessing it
g_connman->Stop();
}
peerLogic.reset();
g_connman.reset();

if (!fLiteMode && !fRPCInWarmup) {
Expand All @@ -263,7 +265,6 @@ void PrepareShutdown()
flatdb6.Dump(sporkManager);
}

UnregisterNodeSignals(GetNodeSignals());
if (fDumpMempoolLater && gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
DumpMempool();
}
Expand Down Expand Up @@ -1588,7 +1589,6 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)

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 @@ -2127,6 +2127,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
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
20 changes: 9 additions & 11 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,6 @@ std::string strSubVersion;

unordered_limitedmap<uint256, int64_t, StaticSaltedHasher> mapAlreadyAskedFor(MAX_INV_SZ, MAX_INV_SZ * 2);

// 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 @@ -1201,7 +1197,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true);
pnode->AddRef();
pnode->fWhitelisted = whitelisted;
GetNodeSignals().InitializeNode(pnode, *this);
m_msgproc->InitializeNode(pnode);

if (fLogIPs) {
LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString());
Expand Down Expand Up @@ -2187,7 +2183,7 @@ bool CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
if (fConnectToMasternode)
pnode->fMasternode = true;

GetNodeSignals().InitializeNode(pnode, *this);
m_msgproc->InitializeNode(pnode);
{
LOCK(cs_vNodes);
vNodes.push_back(pnode);
Expand All @@ -2214,16 +2210,16 @@ 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 @@ -2546,6 +2542,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
//
// Start threads
//
assert(m_msgproc);
InterruptSocks5(false);
interruptNet.reset();
flagInterruptMsgProc = false;
Expand Down Expand Up @@ -2709,9 +2706,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
25 changes: 13 additions & 12 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include <arpa/inet.h>
#endif

#include <boost/signals2/signal.hpp>

// "Optimistic send" was introduced in the beginning of the Bitcoin project. I assume this was done because it was
// thought that "send" would be very cheap when the send buffer is empty. This is not true, as shown by profiling.
Expand Down Expand Up @@ -140,7 +139,7 @@ struct CSerializedNetMsg
std::string command;
};


class NetEventsInterface;
class CConnman
{
public:
Expand All @@ -161,6 +160,7 @@ class CConnman
int nMaxFeeler = 0;
int nBestHeight = 0;
CClientUIInterface* uiInterface = nullptr;
NetEventsInterface* m_msgproc = nullptr;
unsigned int nSendBufferMaxSize = 0;
unsigned int nReceiveFloodSize = 0;
uint64_t nMaxOutboundTimeframe = 0;
Expand All @@ -178,6 +178,7 @@ class CConnman
nMaxFeeler = connOptions.nMaxFeeler;
nBestHeight = connOptions.nBestHeight;
clientInterface = connOptions.uiInterface;
m_msgproc = connOptions.m_msgproc;
nSendBufferMaxSize = connOptions.nSendBufferMaxSize;
nReceiveFloodSize = connOptions.nReceiveFloodSize;
nMaxOutboundTimeframe = connOptions.nMaxOutboundTimeframe;
Expand Down Expand Up @@ -537,6 +538,7 @@ class CConnman
int nMaxFeeler;
std::atomic<int> nBestHeight;
CClientUIInterface* clientInterface;
NetEventsInterface* m_msgproc;

/** SipHasher seeds for deterministic randomness */
const uint64_t nSeed0, nSeed1;
Expand Down Expand Up @@ -584,19 +586,18 @@ 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
Expand Down
Loading

0 comments on commit 022e80a

Please sign in to comment.