Skip to content

Commit

Permalink
Backport 12381 (dashpay#3528)
Browse files Browse the repository at this point in the history 
* Merge bitcoin#12381: Remove more boost threads

004f999 boost: drop boost threads for [alert|block|wallet]notify (Cory Fields)
0827267 boost: drop boost threads from torcontrol (Cory Fields)
ba91724 boost: remove useless threadGroup parameter from Discover (Cory Fields)
f26866b boost: drop boost threads for upnp (Cory Fields)

Pull request description:

  This doesn't completely get rid of boost::thread, but this batch should be easy to review, and leaves us with only threadGroup (scheduler + scriptcheck) remaining.

  Note to reviewers: The upnp diff changes a bunch of whitespace, it's much more clear with 'git diff -w'

Tree-SHA512: 5a356798d0785f93ed143d1f0afafe890bc82f0d470bc969473da2d2aa78bcb9b096f7ba11b92564d546fb447d4bd0d347e7842994ea0170aafd53fda7e0a66e

* fix using std::thread

Signed-off-by: pasta <[email protected]>

* Switch to std::thread in NotifyTransactionLock

* Move StopTorControl call from Shutdown to PrepareShutdown

Co-authored-by: Wladimir J. van der Laan <[email protected]>
Co-authored-by: UdjinM6 <[email protected]>
  • Loading branch information
@PastaPastaPasta @laanwj @UdjinM6
3 people authored Jun 13, 2020
1 parent de43cf0 commit 224d0a3
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 68 deletions.
19 changes: 12 additions & 7 deletions src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ void Interrupt()
InterruptREST();
InterruptTorControl();
llmq::InterruptLLMQSystem();
InterruptMapPort();
if (g_connman)
g_connman->Interrupt();
}
Expand Down Expand Up @@ -251,14 +252,16 @@ void PrepareShutdown()
bool fRPCInWarmup = RPCIsInWarmup(&statusmessage);

g_wallet_init_interface->Flush();
MapPort(false);
StopMapPort();

// Because these depend on each-other, we make sure that neither can be
// using the other before destroying them.
if (peerLogic) UnregisterValidationInterface(peerLogic.get());
if (g_connman) g_connman->Stop();
// if (g_txindex) g_txindex->Stop(); //TODO watch out when backporting bitcoin#13033 (don't accidently put the reset here, as we've already backported bitcoin#13894)

StopTorControl();

// After everything has been shut down, but before things get flushed, stop the
// CScheduler/checkqueue threadGroup
threadGroup.interrupt_all();
Expand Down Expand Up @@ -378,8 +381,7 @@ void Shutdown()
if(!fRequestRestart) {
PrepareShutdown();
}
// Shutdown part 2: Stop TOR thread and delete wallet instance
StopTorControl();
// Shutdown part 2: delete wallet instance
g_wallet_init_interface->Close();
globalVerifyHandle.reset();
ECC_Stop();
Expand Down Expand Up @@ -695,7 +697,8 @@ static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex
std::string strCmd = gArgs.GetArg("-blocknotify", "");
if (!strCmd.empty()) {
boost::replace_all(strCmd, "%s", pBlockIndex->GetBlockHash().GetHex());
boost::thread t(runCommand, strCmd); // thread runs free
std::thread t(runCommand, strCmd);
t.detach(); // thread runs free
}
}

Expand Down Expand Up @@ -2221,12 +2224,14 @@ bool AppInitMain()
}
LogPrintf("chainActive.Height() = %d\n", chain_active_height);
if (gArgs.GetBoolArg("-listenonion", DEFAULT_LISTEN_ONION))
StartTorControl(threadGroup, scheduler);
StartTorControl();

Discover(threadGroup);
Discover();

// Map ports with UPnP
MapPort(gArgs.GetBoolArg("-upnp", DEFAULT_UPNP));
if (gArgs.GetBoolArg("-upnp", DEFAULT_UPNP)) {
StartMapPort();
}

CConnman::Options connOptions;
connOptions.nLocalServices = nLocalServices;
Expand Down
86 changes: 47 additions & 39 deletions src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1948,6 +1948,8 @@ void CConnman::WakeSelect()


#ifdef USE_UPNP
static CThreadInterrupt g_upnp_interrupt;
static std::thread g_upnp_thread;
void ThreadMapPort()
{
std::string port = strprintf("%u", GetListenPort());
Expand Down Expand Up @@ -1998,35 +2000,29 @@ void ThreadMapPort()

std::string strDesc = "Dash Core " + FormatFullVersion();

try {
while (true) {
do {
#ifndef UPNPDISCOVER_SUCCESS
/* miniupnpc 1.5 */
r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype,
port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0);
/* miniupnpc 1.5 */
r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype,
port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0);
#else
/* miniupnpc 1.6 */
r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype,
port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0, "0");
/* miniupnpc 1.6 */
r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype,
port.c_str(), port.c_str(), lanaddr, strDesc.c_str(), "TCP", 0, "0");
#endif

if(r!=UPNPCOMMAND_SUCCESS)
LogPrintf("AddPortMapping(%s, %s, %s) failed with code %d (%s)\n",
port, port, lanaddr, r, strupnperror(r));
else
LogPrintf("UPnP Port Mapping successful.\n");

MilliSleep(20*60*1000); // Refresh every 20 minutes
}
}
catch (const boost::thread_interrupted&)
{
r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", 0);
LogPrintf("UPNP_DeletePortMapping() returned: %d\n", r);
freeUPNPDevlist(devlist); devlist = nullptr;
FreeUPNPUrls(&urls);
throw;
if(r!=UPNPCOMMAND_SUCCESS)
LogPrintf("AddPortMapping(%s, %s, %s) failed with code %d (%s)\n",
port, port, lanaddr, r, strupnperror(r));
else
LogPrintf("UPnP Port Mapping successful.\n");
}
while(g_upnp_interrupt.sleep_for(std::chrono::minutes(20)));

r = UPNP_DeletePortMapping(urls.controlURL, data.first.servicetype, port.c_str(), "TCP", 0);
LogPrintf("UPNP_DeletePortMapping() returned: %d\n", r);
freeUPNPDevlist(devlist); devlist = nullptr;
FreeUPNPUrls(&urls);
} else {
LogPrintf("No valid UPnP IGDs found\n");
freeUPNPDevlist(devlist); devlist = nullptr;
Expand All @@ -2035,27 +2031,39 @@ void ThreadMapPort()
}
}

void MapPort(bool fUseUPnP)
void StartMapPort()
{
static std::unique_ptr<boost::thread> upnp_thread;
if (!g_upnp_thread.joinable()) {
assert(!g_upnp_interrupt);
g_upnp_thread = std::thread((std::bind(&TraceThread<void (*)()>, "upnp", &ThreadMapPort)));
}
}

if (fUseUPnP)
{
if (upnp_thread) {
upnp_thread->interrupt();
upnp_thread->join();
}
upnp_thread.reset(new boost::thread(boost::bind(&TraceThread<void (*)()>, "upnp", &ThreadMapPort)));
void InterruptMapPort()
{
if(g_upnp_thread.joinable()) {
g_upnp_interrupt();
}
else if (upnp_thread) {
upnp_thread->interrupt();
upnp_thread->join();
upnp_thread.reset();
}

void StopMapPort()
{
if(g_upnp_thread.joinable()) {
g_upnp_thread.join();
g_upnp_interrupt.reset();
}
}

#else
void MapPort(bool)
void StartMapPort()
{
// Intentionally left blank.
}
void InterruptMapPort()
{
// Intentionally left blank.
}
void StopMapPort()
{
// Intentionally left blank.
}
Expand Down Expand Up @@ -2827,7 +2835,7 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b
return true;
}

void Discover(boost::thread_group& threadGroup)
void Discover()
{
if (!fDiscover)
return;
Expand Down
10 changes: 4 additions & 6 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@
class CScheduler;
class CNode;

namespace boost {
class thread_group;
} // namespace boost

/** Time between pings automatically sent out for latency probing and keepalive (in seconds). */
static const int PING_INTERVAL = 2 * 60;
/** Time after which to disconnect, after waiting for a ping response (or inactivity). */
Expand Down Expand Up @@ -651,8 +647,10 @@ friend class CNode;
friend struct CConnmanTest;
};
extern std::unique_ptr<CConnman> g_connman;
void Discover(boost::thread_group& threadGroup);
void MapPort(bool fUseUPnP);
void Discover();
void StartMapPort();
void InterruptMapPort();
void StopMapPort();
unsigned short GetListenPort();
bool BindListenPort(const CService &bindAddr, std::string& strError, bool fWhitelisted = false);

Expand Down
7 changes: 6 additions & 1 deletion src/qt/optionsmodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,12 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
break;
case MapPortUPnP: // core option - can be changed on-the-fly
settings.setValue("fUseUPnP", value.toBool());
MapPort(value.toBool());
if (value.toBool()) {
StartMapPort();
} else {
InterruptMapPort();
StopMapPort();
}
break;
case MinimizeOnClose:
fMinimizeOnClose = value.toBool();
Expand Down
14 changes: 4 additions & 10 deletions src/torcontrol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ void TorController::reconnect_cb(evutil_socket_t fd, short what, void *arg)

/****** Thread ********/
static struct event_base *gBase;
static boost::thread torControlThread;
static std::thread torControlThread;

static void TorControlThread()
{
Expand All @@ -740,7 +740,7 @@ static void TorControlThread()
event_base_dispatch(gBase);
}

void StartTorControl(boost::thread_group& threadGroup, CScheduler& scheduler)
void StartTorControl()
{
assert(!gBase);
#ifdef WIN32
Expand All @@ -754,7 +754,7 @@ void StartTorControl(boost::thread_group& threadGroup, CScheduler& scheduler)
return;
}

torControlThread = boost::thread(boost::bind(&TraceThread<void (*)()>, "torcontrol", &TorControlThread));
torControlThread = std::thread(std::bind(&TraceThread<void (*)()>, "torcontrol", &TorControlThread));
}

void InterruptTorControl()
Expand All @@ -767,14 +767,8 @@ void InterruptTorControl()

void StopTorControl()
{
// timed_join() avoids the wallet not closing during a repair-restart. For a 'normal' wallet exit
// it behaves for our cases exactly like the normal join()
if (gBase) {
#if BOOST_VERSION >= 105000
torControlThread.try_join_for(boost::chrono::seconds(1));
#else
torControlThread.timed_join(boost::posix_time::seconds(1));
#endif
torControlThread.join();
event_base_free(gBase);
gBase = nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion src/torcontrol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
extern const std::string DEFAULT_TOR_CONTROL;
static const bool DEFAULT_LISTEN_ONION = true;

void StartTorControl(boost::thread_group& threadGroup, CScheduler& scheduler);
void StartTorControl();
void InterruptTorControl();
void StopTorControl();

Expand Down
3 changes: 2 additions & 1 deletion src/validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,8 @@ static void AlertNotify(const std::string& strMessage)
safeStatus = singleQuote+safeStatus+singleQuote;
boost::replace_all(strCmd, "%s", safeStatus);

boost::thread t(runCommand, strCmd); // thread runs free
std::thread t(runCommand, strCmd);
t.detach(); // thread runs free
}

static void CheckForkWarningConditions()
Expand Down
7 changes: 4 additions & 3 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#include <future>

#include <boost/algorithm/string/replace.hpp>
#include <boost/thread.hpp>

static CCriticalSection cs_wallets;
static std::vector<CWallet*> vpwallets GUARDED_BY(cs_wallets);
Expand Down Expand Up @@ -1169,7 +1168,8 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
if (!strCmd.empty())
{
boost::replace_all(strCmd, "%s", wtxIn.GetHash().GetHex());
boost::thread t(runCommand, strCmd); // thread runs free
std::thread t(runCommand, strCmd);
t.detach(); // thread runs free
}

fAnonymizableTallyCached = false;
Expand Down Expand Up @@ -5439,7 +5439,8 @@ void CWallet::NotifyTransactionLock(const CTransaction &tx, const llmq::CInstant
std::string strCmd = gArgs.GetArg("-instantsendnotify", "");
if (!strCmd.empty()) {
boost::replace_all(strCmd, "%s", txHash.GetHex());
boost::thread t(runCommand, strCmd); // thread runs free
std::thread t(runCommand, strCmd);
t.detach(); // thread runs free
}
}
}
Expand Down

0 comments on commit 224d0a3

Please sign in to comment.