Skip to content

Commit

Permalink
refactor: remove boost::thread_group usage
Browse files Browse the repository at this point in the history
  • Loading branch information
fanquake committed Jan 29, 2021
1 parent c8b8351 commit dc8be12
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 39 deletions.
2 changes: 2 additions & 0 deletions src/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <logging.h>
#include <serialize.h>

#include <cmath>

int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const
{
uint64_t hash1 = (CHashWriter(SER_GETHASH, 0) << nKey << GetKey()).GetCheapHash();
Expand Down
1 change: 1 addition & 0 deletions src/bitcoin-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <util/url.h>

#include <algorithm>
#include <cmath>
#include <functional>
#include <memory>
#include <stdio.h>
Expand Down
11 changes: 4 additions & 7 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@
#include <set>
#include <stdint.h>
#include <stdio.h>
#include <thread>
#include <vector>

#ifndef WIN32
#include <attributes.h>
Expand All @@ -78,7 +80,6 @@

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

#if ENABLE_ZMQ
#include <zmq/zmqabstractnotifier.h>
Expand Down Expand Up @@ -155,8 +156,6 @@ static std::unique_ptr<ECCVerifyHandle> globalVerifyHandle;

static std::thread g_load_block;

static boost::thread_group threadGroup;

void Interrupt(NodeContext& node)
{
InterruptHTTPServer();
Expand Down Expand Up @@ -218,11 +217,9 @@ void Shutdown(NodeContext& node)
StopTorControl();

// After everything has been shut down, but before things get flushed, stop the
// CScheduler/checkqueue, threadGroup and load block thread.
// CScheduler/checkqueue, scheduler and load block thread.
if (node.scheduler) node.scheduler->stop();
if (g_load_block.joinable()) g_load_block.join();
threadGroup.interrupt_all();
threadGroup.join_all();
StopScriptCheckWorkerThreads();

// After the threads that potentially access these pointers have been stopped,
Expand Down Expand Up @@ -1342,7 +1339,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA
node.scheduler = MakeUnique<CScheduler>();

// Start the lightweight task scheduler thread
threadGroup.create_thread([&] { TraceThread("scheduler", [&] { node.scheduler->serviceQueue(); }); });
node.scheduler->m_service_thread = std::thread([&] { TraceThread("scheduler", [&] { node.scheduler->serviceQueue(); }); });

// Gather some entropy once per minute.
node.scheduler->scheduleEvery([]{
Expand Down
8 changes: 6 additions & 2 deletions src/scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <functional>
#include <list>
#include <map>
#include <thread>

#include <sync.h>

Expand All @@ -35,6 +36,8 @@ class CScheduler
CScheduler();
~CScheduler();

std::thread m_service_thread;

typedef std::function<void()> Function;

/** Call func at/after time t */
Expand Down Expand Up @@ -62,8 +65,7 @@ class CScheduler
void MockForward(std::chrono::seconds delta_seconds);

/**
* Services the queue 'forever'. Should be run in a thread,
* and interrupted using boost::interrupt_thread
* Services the queue 'forever'. Should be run in a thread.
*/
void serviceQueue();

Expand All @@ -72,12 +74,14 @@ class CScheduler
{
WITH_LOCK(newTaskMutex, stopRequested = true);
newTaskScheduled.notify_all();
if (m_service_thread.joinable()) m_service_thread.join();
}
/** Tell any threads running serviceQueue to stop when there is no work left to be done */
void StopWhenDrained()
{
WITH_LOCK(newTaskMutex, stopWhenEmpty = true);
newTaskScheduled.notify_all();
if (m_service_thread.joinable()) m_service_thread.join();
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/script/sigcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include <util/system.h>

#include <cuckoocache.h>

#include <boost/thread/lock_types.hpp>
#include <boost/thread/shared_mutex.hpp>

namespace {
Expand Down
17 changes: 10 additions & 7 deletions src/test/checkqueue_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include <util/time.h>

#include <boost/test/unit_test.hpp>
#include <boost/thread/thread.hpp>

#include <atomic>
#include <condition_variable>
Expand Down Expand Up @@ -363,11 +362,11 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
{
auto queue = MakeUnique<Standard_Queue>(QUEUE_BATCH_SIZE);
{
boost::thread_group tg;
std::vector<std::thread> tg;
std::atomic<int> nThreads {0};
std::atomic<int> fails {0};
for (size_t i = 0; i < 3; ++i) {
tg.create_thread(
tg.emplace_back(
[&]{
CCheckQueueControl<FakeCheck> control(queue.get());
// While sleeping, no other thread should execute to this point
Expand All @@ -376,11 +375,13 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
fails += observed != nThreads;
});
}
tg.join_all();
for (auto& thread: tg) {
if (thread.joinable()) thread.join();
}
BOOST_REQUIRE_EQUAL(fails, 0);
}
{
boost::thread_group tg;
std::vector<std::thread> tg;
std::mutex m;
std::condition_variable cv;
bool has_lock{false};
Expand All @@ -389,7 +390,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
bool done_ack{false};
{
std::unique_lock<std::mutex> l(m);
tg.create_thread([&]{
tg.emplace_back([&]{
CCheckQueueControl<FakeCheck> control(queue.get());
std::unique_lock<std::mutex> ll(m);
has_lock = true;
Expand All @@ -415,7 +416,9 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
cv.notify_one();
BOOST_REQUIRE(!fails);
}
tg.join_all();
for (auto& thread: tg) {
if (thread.joinable()) thread.join();
}
}
}
BOOST_AUTO_TEST_SUITE_END()
2 changes: 2 additions & 0 deletions src/test/cuckoocache_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <boost/test/unit_test.hpp>
#include <boost/thread/lock_types.hpp>
#include <boost/thread/shared_mutex.hpp>
#include <cuckoocache.h>
#include <deque>
#include <random.h>
Expand Down
22 changes: 14 additions & 8 deletions src/test/scheduler_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
#include <util/time.h>

#include <boost/test/unit_test.hpp>
#include <boost/thread/thread.hpp>

#include <functional>
#include <mutex>
#include <thread>
#include <vector>

BOOST_AUTO_TEST_SUITE(scheduler_tests)

Expand Down Expand Up @@ -69,16 +70,16 @@ BOOST_AUTO_TEST_CASE(manythreads)
BOOST_CHECK(last > now);

// As soon as these are created they will start running and servicing the queue
boost::thread_group microThreads;
std::vector<std::thread> microThreads;
for (int i = 0; i < 5; i++)
microThreads.create_thread(std::bind(&CScheduler::serviceQueue, &microTasks));
microThreads.emplace_back(std::bind(&CScheduler::serviceQueue, &microTasks));

UninterruptibleSleep(std::chrono::microseconds{600});
now = std::chrono::system_clock::now();

// More threads and more tasks:
for (int i = 0; i < 5; i++)
microThreads.create_thread(std::bind(&CScheduler::serviceQueue, &microTasks));
microThreads.emplace_back(std::bind(&CScheduler::serviceQueue, &microTasks));
for (int i = 0; i < 100; i++) {
std::chrono::system_clock::time_point t = now + std::chrono::microseconds(randomMsec(rng));
std::chrono::system_clock::time_point tReschedule = now + std::chrono::microseconds(500 + randomMsec(rng));
Expand All @@ -91,7 +92,10 @@ BOOST_AUTO_TEST_CASE(manythreads)

// Drain the task queue then exit threads
microTasks.StopWhenDrained();
microThreads.join_all(); // ... wait until all the threads are done
// wait until all the threads are done
for (auto& thread: microThreads) {
if (thread.joinable()) thread.join();
}

int counterSum = 0;
for (int i = 0; i < 10; i++) {
Expand Down Expand Up @@ -131,9 +135,9 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered)
// if the queues only permit execution of one task at once then
// the extra threads should effectively be doing nothing
// if they don't we'll get out of order behaviour
boost::thread_group threads;
std::vector<std::thread> threads;
for (int i = 0; i < 5; ++i) {
threads.create_thread(std::bind(&CScheduler::serviceQueue, &scheduler));
threads.emplace_back(std::bind(&CScheduler::serviceQueue, &scheduler));
}

// these are not atomic, if SinglethreadedSchedulerClient prevents
Expand All @@ -157,7 +161,9 @@ BOOST_AUTO_TEST_CASE(singlethreadedscheduler_ordered)

// finish up
scheduler.StopWhenDrained();
threads.join_all();
for (auto& thread: threads) {
if (thread.joinable()) thread.join();
}

BOOST_CHECK_EQUAL(counter1, 100);
BOOST_CHECK_EQUAL(counter2, 100);
Expand Down
4 changes: 1 addition & 3 deletions src/test/util/setup_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
// We have to run a scheduler thread to prevent ActivateBestChain
// from blocking due to queue overrun.
m_node.scheduler = MakeUnique<CScheduler>();
threadGroup.create_thread([&] { TraceThread("scheduler", [&] { m_node.scheduler->serviceQueue(); }); });
m_node.scheduler->m_service_thread = std::thread([&] { TraceThread("scheduler", [&] { m_node.scheduler->serviceQueue(); }); });
GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler);

pblocktree.reset(new CBlockTreeDB(1 << 20, true));
Expand All @@ -150,8 +150,6 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
ChainTestingSetup::~ChainTestingSetup()
{
if (m_node.scheduler) m_node.scheduler->stop();
threadGroup.interrupt_all();
threadGroup.join_all();
StopScriptCheckWorkerThreads();
GetMainSignals().FlushBackgroundCallbacks();
GetMainSignals().UnregisterBackgroundSignalScheduler();
Expand Down
4 changes: 1 addition & 3 deletions src/test/util/setup_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
#include <util/string.h>

#include <type_traits>

#include <boost/thread/thread.hpp>
#include <vector>

/** This is connected to the logger. Can be used to redirect logs to any other log */
extern const std::function<void(const std::string&)> G_TEST_LOG_FUN;
Expand Down Expand Up @@ -88,7 +87,6 @@ struct BasicTestingSetup {
* initialization behaviour.
*/
struct ChainTestingSetup : public BasicTestingSetup {
boost::thread_group threadGroup;

explicit ChainTestingSetup(const std::string& chainName = CBaseChainParams::MAIN, const std::vector<const char*>& extra_args = {});
~ChainTestingSetup();
Expand Down
7 changes: 0 additions & 7 deletions src/util/system.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@
#include <utility>
#include <vector>

#include <boost/thread/condition_variable.hpp> // for boost::thread_interrupted

class UniValue;

// Application startup time (used for uptime calculation)
Expand Down Expand Up @@ -450,11 +448,6 @@ template <typename Callable> void TraceThread(const char* name, Callable func)
func();
LogPrintf("%s thread exit\n", name);
}
catch (const boost::thread_interrupted&)
{
LogPrintf("%s thread interrupt\n", name);
throw;
}
catch (const std::exception& e) {
PrintExceptionContinue(&e, name);
throw;
Expand Down
3 changes: 1 addition & 2 deletions test/lint/lint-includes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ EXPECTED_BOOST_INCLUDES=(
boost/signals2/optional_last_value.hpp
boost/signals2/signal.hpp
boost/test/unit_test.hpp
boost/thread/condition_variable.hpp
boost/thread/lock_types.hpp
boost/thread/shared_mutex.hpp
boost/thread/thread.hpp
)

for BOOST_INCLUDE in $(git grep '^#include <boost/' -- "*.cpp" "*.h" | cut -f2 -d: | cut -f2 -d'<' | cut -f1 -d'>' | sort -u); do
Expand Down

0 comments on commit dc8be12

Please sign in to comment.