Skip to content

Commit

Permalink
Merge bitcoin#16117: util: Replace boost sleep with std sleep
Browse files Browse the repository at this point in the history
fae86c3 util: Remove unused MilliSleep (MarcoFalke)
fa9af06 scripted-diff: Replace MilliSleep with UninterruptibleSleep (MarcoFalke)
fa4620b util: Add UnintrruptibleSleep (MarcoFalke)

Pull request description:

  We don't use the interruptible feature of boost's sleep anywhere, so replace it with the sleep in `std::thread`

ACKs for top commit:
  ajtowns:
    ACK fae86c3 quick code review
  practicalswift:
    ACK fae86c3 -- patch looks correct
  sipa:
    Concept and code review ACK fae86c3
  fanquake:
    ACK fae86c3 - note that an instance of `DHAVE_WORKING_BOOST_SLEEP_FOR` was missed in the [linter](https://github.com/bitcoin/bitcoin/blob/master/test/lint/extended-lint-cppcheck.sh#L69), but that can be cleaned up later.

Tree-SHA512: 7c0f8eb197664b9f7d9fe6c472c77d384f11c797c913afc31de4b532e3b4fd9ea6dd174f92062ff9d1ec39b25e0900ca7c597435add87f0f2477d9557204848c
  • Loading branch information
fanquake committed Mar 6, 2020
2 parents 3f82659 + fae86c3 commit 97aadf9
Show file tree
Hide file tree
Showing 16 changed files with 21 additions and 104 deletions.
6 changes: 0 additions & 6 deletions build_msvc/bitcoin_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,6 @@
/* Define if the visibility attribute is supported. */
#define HAVE_VISIBILITY_ATTRIBUTE 1

/* Define this symbol if boost sleep works */
/* #undef HAVE_WORKING_BOOST_SLEEP */

/* Define this symbol if boost sleep_for works */
#define HAVE_WORKING_BOOST_SLEEP_FOR 1

/* Define to the sub-directory where libtool stores uninstalled libraries. */
#define LT_OBJDIR ".libs/"

Expand Down
51 changes: 0 additions & 51 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -1244,57 +1244,6 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([[
LIBS="$TEMP_LIBS"
CPPFLAGS="$TEMP_CPPFLAGS"

dnl Boost >= 1.50 uses sleep_for rather than the now-deprecated sleep, however
dnl it was broken from 1.50 to 1.52 when backed by nanosleep. Use sleep_for if
dnl a working version is available, else fall back to sleep. sleep was removed
dnl after 1.56.
dnl If neither is available, abort.
TEMP_LIBS="$LIBS"
LIBS="$BOOST_LIBS $LIBS"
TEMP_CPPFLAGS="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS"
AC_LINK_IFELSE([AC_LANG_PROGRAM([[
#include <boost/thread/thread.hpp>
#include <boost/version.hpp>
]],[[
#if BOOST_VERSION >= 105000 && (!defined(BOOST_HAS_NANOSLEEP) || BOOST_VERSION >= 105200)
boost::this_thread::sleep_for(boost::chrono::milliseconds(0));
#else
choke me
#endif
]])],
[boost_sleep=yes;
AC_DEFINE(HAVE_WORKING_BOOST_SLEEP_FOR, 1, [Define this symbol if boost sleep_for works])],
[boost_sleep=no])
LIBS="$TEMP_LIBS"
CPPFLAGS="$TEMP_CPPFLAGS"

if test x$boost_sleep != xyes; then
TEMP_LIBS="$LIBS"
LIBS="$BOOST_LIBS $LIBS"
TEMP_CPPFLAGS="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS"
AC_LINK_IFELSE([AC_LANG_PROGRAM([[
#include <boost/version.hpp>
#include <boost/thread.hpp>
#include <boost/date_time/posix_time/posix_time_types.hpp>
]],[[
#if BOOST_VERSION <= 105600
boost::this_thread::sleep(boost::posix_time::milliseconds(0));
#else
choke me
#endif
]])],
[boost_sleep=yes; AC_DEFINE(HAVE_WORKING_BOOST_SLEEP, 1, [Define this symbol if boost sleep works])],
[boost_sleep=no])
LIBS="$TEMP_LIBS"
CPPFLAGS="$TEMP_CPPFLAGS"
fi

if test x$boost_sleep != xyes; then
AC_MSG_ERROR(No working boost sleep implementation found.)
fi

fi

if test x$use_pkgconfig = xyes; then
Expand Down
2 changes: 1 addition & 1 deletion src/bench/examples.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
static void Sleep100ms(benchmark::State& state)
{
while (state.KeepRunning()) {
MilliSleep(100);
UninterruptibleSleep(std::chrono::milliseconds{100});
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/bitcoin-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ static int CommandLineRPC(int argc, char *argv[])
}
catch (const CConnectionFailed&) {
if (fWait)
MilliSleep(1000);
UninterruptibleSleep(std::chrono::milliseconds{1000});
else
throw;
}
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ static void WaitForShutdown(NodeContext& node)
{
while (!ShutdownRequested())
{
MilliSleep(200);
UninterruptibleSleep(std::chrono::milliseconds{200});
}
Interrupt(node);
}
Expand Down
2 changes: 1 addition & 1 deletion src/httprpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ static bool HTTPReq_JSONRPC(HTTPRequest* req, const std::string &)
/* Deter brute-forcing
If this results in a DoS the user really
shouldn't have their RPC port exposed. */
MilliSleep(250);
UninterruptibleSleep(std::chrono::milliseconds{250});

req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA);
req->WriteReply(HTTP_UNAUTHORIZED);
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ UniValue stop(const JSONRPCRequest& jsonRequest)
// this reply will get back to the client.
StartShutdown();
if (jsonRequest.params[0].isNum()) {
MilliSleep(jsonRequest.params[0].get_int());
UninterruptibleSleep(std::chrono::milliseconds{jsonRequest.params[0].get_int()});
}
return PACKAGE_NAME " stopping";
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/blockfilter_index_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
int64_t time_start = GetTimeMillis();
while (!filter_index.BlockUntilSyncedToCurrentChain()) {
BOOST_REQUIRE(time_start + timeout_ms > GetTimeMillis());
MilliSleep(100);
UninterruptibleSleep(std::chrono::milliseconds{100});
}

// Check that filter index has all blocks that were in the chain before it started.
Expand Down
2 changes: 1 addition & 1 deletion src/test/checkqueue_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)
CCheckQueueControl<FakeCheck> control(queue.get());
// While sleeping, no other thread should execute to this point
auto observed = ++nThreads;
MilliSleep(10);
UninterruptibleSleep(std::chrono::milliseconds{10});
fails += observed != nThreads;
});
}
Expand Down
15 changes: 2 additions & 13 deletions src/test/scheduler_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <random.h>
#include <scheduler.h>
#include <util/time.h>

#include <boost/thread.hpp>
#include <boost/test/unit_test.hpp>
Expand All @@ -23,18 +24,6 @@ static void microTask(CScheduler& s, boost::mutex& mutex, int& counter, int delt
}
}

static void MicroSleep(uint64_t n)
{
#if defined(HAVE_WORKING_BOOST_SLEEP_FOR)
boost::this_thread::sleep_for(boost::chrono::microseconds(n));
#elif defined(HAVE_WORKING_BOOST_SLEEP)
boost::this_thread::sleep(boost::posix_time::microseconds(n));
#else
//should never get here
#error missing boost sleep implementation
#endif
}

BOOST_AUTO_TEST_CASE(manythreads)
{
// Stress test: hundreds of microsecond-scheduled tasks,
Expand Down Expand Up @@ -81,7 +70,7 @@ BOOST_AUTO_TEST_CASE(manythreads)
for (int i = 0; i < 5; i++)
microThreads.create_thread(std::bind(&CScheduler::serviceQueue, &microTasks));

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

// More threads and more tasks:
Expand Down
2 changes: 1 addition & 1 deletion src/test/txindex_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
int64_t time_start = GetTimeMillis();
while (!txindex.BlockUntilSyncedToCurrentChain()) {
BOOST_REQUIRE(time_start + timeout_ms > GetTimeMillis());
MilliSleep(100);
UninterruptibleSleep(std::chrono::milliseconds{100});
}

// Check that txindex excludes genesis block transactions.
Expand Down
4 changes: 2 additions & 2 deletions src/test/util_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,7 @@ BOOST_AUTO_TEST_CASE(util_time_GetTime)
SetMockTime(111);
// Check that mock time does not change after a sleep
for (const auto& num_sleep : {0, 1}) {
MilliSleep(num_sleep);
UninterruptibleSleep(std::chrono::milliseconds{num_sleep});
BOOST_CHECK_EQUAL(111, GetTime()); // Deprecated time getter
BOOST_CHECK_EQUAL(111, GetTime<std::chrono::seconds>().count());
BOOST_CHECK_EQUAL(111000, GetTime<std::chrono::milliseconds>().count());
Expand All @@ -1344,7 +1344,7 @@ BOOST_AUTO_TEST_CASE(util_time_GetTime)
// Check that system time changes after a sleep
const auto ms_0 = GetTime<std::chrono::milliseconds>();
const auto us_0 = GetTime<std::chrono::microseconds>();
MilliSleep(1);
UninterruptibleSleep(std::chrono::milliseconds{1});
BOOST_CHECK(ms_0 < GetTime<std::chrono::milliseconds>());
BOOST_CHECK(us_0 < GetTime<std::chrono::microseconds>());
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/validation_block_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
t.join();
}
while (GetMainSignals().CallbacksPending() > 0) {
MilliSleep(100);
UninterruptibleSleep(std::chrono::milliseconds{100});
}

UnregisterValidationInterface(&sub);
Expand Down
23 changes: 4 additions & 19 deletions src/util/time.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@

#include <atomic>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/thread.hpp>
#include <ctime>
#include <thread>

#include <tinyformat.h>

void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); }

static std::atomic<int64_t> nMockTime(0); //!< For unit testing

int64_t GetTime()
Expand Down Expand Up @@ -72,24 +75,6 @@ int64_t GetSystemTimeInSeconds()
return GetTimeMicros()/1000000;
}

void MilliSleep(int64_t n)
{

/**
* Boost's sleep_for was uninterruptible when backed by nanosleep from 1.50
* until fixed in 1.52. Use the deprecated sleep method for the broken case.
* See: https://svn.boost.org/trac/boost/ticket/7238
*/
#if defined(HAVE_WORKING_BOOST_SLEEP_FOR)
boost::this_thread::sleep_for(boost::chrono::milliseconds(n));
#elif defined(HAVE_WORKING_BOOST_SLEEP)
boost::this_thread::sleep(boost::posix_time::milliseconds(n));
#else
//should never get here
#error missing boost sleep implementation
#endif
}

std::string FormatISO8601DateTime(int64_t nTime) {
struct tm ts;
time_t time_val = nTime;
Expand Down
4 changes: 2 additions & 2 deletions src/util/time.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include <string>
#include <chrono>

void UninterruptibleSleep(const std::chrono::microseconds& n);

/**
* Helper to count the seconds of a duration.
*
Expand All @@ -36,8 +38,6 @@ void SetMockTime(int64_t nMockTimeIn);
/** For testing */
int64_t GetMockTime();

void MilliSleep(int64_t n);

/** Return system time (or mocked time, if set) */
template <typename T>
T GetTime();
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ bool BerkeleyBatch::Rewrite(BerkeleyDatabase& database, const char* pszSkip)
return fSuccess;
}
}
MilliSleep(100);
UninterruptibleSleep(std::chrono::milliseconds{100});
}
}

Expand Down Expand Up @@ -887,7 +887,7 @@ bool BerkeleyDatabase::Backup(const std::string& strDest)
}
}
}
MilliSleep(100);
UninterruptibleSleep(std::chrono::milliseconds{100});
}
}

Expand Down

0 comments on commit 97aadf9

Please sign in to comment.