From fa4620be782c2bf6b5ffddf4f671194fdd1536f3 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 21 Feb 2020 09:19:57 -0800 Subject: util: Add UnintrruptibleSleep --- src/util/time.cpp | 6 +++++- src/util/time.h | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/util/time.cpp b/src/util/time.cpp index 2afff2626b..5c1182850e 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -13,8 +13,12 @@ #include #include #include +#include + #include +void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); } + static std::atomic nMockTime(0); //!< For unit testing int64_t GetTime() @@ -124,4 +128,4 @@ int64_t ParseISO8601DateTime(const std::string& str) if (ptime.is_not_a_date_time() || epoch > ptime) return 0; return (ptime - epoch).total_seconds(); -} \ No newline at end of file +} diff --git a/src/util/time.h b/src/util/time.h index af4390aa1c..445ca3acb3 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -10,6 +10,8 @@ #include #include +void UninterruptibleSleep(const std::chrono::microseconds& n); + /** * Helper to count the seconds of a duration. * -- cgit v1.2.3 From fa9af06d91e9357e86863781746f0e78a509967e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 21 Feb 2020 09:20:51 -0800 Subject: scripted-diff: Replace MilliSleep with UninterruptibleSleep This is safe because MilliSleep is never executed in a boost::thread, the only type of thread that is interruptible. * The RPC server uses std::thread * The wallet is either executed in an RPC thread or the main thread * bitcoin-cli, benchmarks and tests are only one thread (the main thread) -BEGIN VERIFY SCRIPT- sed -i --regexp-extended -e 's/MilliSleep\((\S+)\);/UninterruptibleSleep(std::chrono::milliseconds{\1});/g' $(git grep -l MilliSleep) -END VERIFY SCRIPT- --- src/bench/examples.cpp | 2 +- src/bitcoin-cli.cpp | 2 +- src/bitcoind.cpp | 2 +- src/httprpc.cpp | 2 +- src/rpc/server.cpp | 2 +- src/test/blockfilter_index_tests.cpp | 2 +- src/test/checkqueue_tests.cpp | 2 +- src/test/txindex_tests.cpp | 2 +- src/test/util_tests.cpp | 4 ++-- src/test/validation_block_tests.cpp | 2 +- src/wallet/db.cpp | 4 ++-- 11 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/bench/examples.cpp b/src/bench/examples.cpp index 60a4fbf0ba..a2fdab5609 100644 --- a/src/bench/examples.cpp +++ b/src/bench/examples.cpp @@ -10,7 +10,7 @@ static void Sleep100ms(benchmark::State& state) { while (state.KeepRunning()) { - MilliSleep(100); + UninterruptibleSleep(std::chrono::milliseconds{100}); } } diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index c085095a2b..6982eaab61 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -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; } diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 4b5cea4849..e284dce0d5 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -29,7 +29,7 @@ static void WaitForShutdown(NodeContext& node) { while (!ShutdownRequested()) { - MilliSleep(200); + UninterruptibleSleep(std::chrono::milliseconds{200}); } Interrupt(node); } diff --git a/src/httprpc.cpp b/src/httprpc.cpp index ff75789223..4d49736140 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -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); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index b62490ed29..4670a28a1b 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -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"; } diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 79e18cd2c0..5e52dc268f 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -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. diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 482fe3772c..a9628e85f9 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -393,7 +393,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks) CCheckQueueControl 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; }); } diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 4b0214a15a..6474711cad 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -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. diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 42c2c50fa5..949d8e6673 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1322,7 +1322,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().count()); BOOST_CHECK_EQUAL(111000, GetTime().count()); @@ -1333,7 +1333,7 @@ BOOST_AUTO_TEST_CASE(util_time_GetTime) // Check that system time changes after a sleep const auto ms_0 = GetTime(); const auto us_0 = GetTime(); - MilliSleep(1); + UninterruptibleSleep(std::chrono::milliseconds{1}); BOOST_CHECK(ms_0 < GetTime()); BOOST_CHECK(us_0 < GetTime()); } diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index dae389a167..f2c862011d 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -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); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 8b042162d8..aa6e91c05e 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -756,7 +756,7 @@ bool BerkeleyBatch::Rewrite(BerkeleyDatabase& database, const char* pszSkip) return fSuccess; } } - MilliSleep(100); + UninterruptibleSleep(std::chrono::milliseconds{100}); } } @@ -887,7 +887,7 @@ bool BerkeleyDatabase::Backup(const std::string& strDest) } } } - MilliSleep(100); + UninterruptibleSleep(std::chrono::milliseconds{100}); } } -- cgit v1.2.3 From fae86c38bca5c960462e53975314a0749db5d17d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 21 Feb 2020 09:48:27 -0800 Subject: util: Remove unused MilliSleep --- build_msvc/bitcoin_config.h | 6 ------ configure.ac | 51 -------------------------------------------- src/test/scheduler_tests.cpp | 15 ++----------- src/util/time.cpp | 19 ----------------- src/util/time.h | 2 -- 5 files changed, 2 insertions(+), 91 deletions(-) diff --git a/build_msvc/bitcoin_config.h b/build_msvc/bitcoin_config.h index 3178f2a3d8..47c0a5f407 100644 --- a/build_msvc/bitcoin_config.h +++ b/build_msvc/bitcoin_config.h @@ -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/" diff --git a/configure.ac b/configure.ac index d0fcf48713..e757940c3e 100644 --- a/configure.ac +++ b/configure.ac @@ -1231,57 +1231,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 - #include - ]],[[ - #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 - #include - #include - ]],[[ - #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 diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index a6cb34cf28..058dd1deda 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -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, @@ -81,7 +70,7 @@ BOOST_AUTO_TEST_CASE(manythreads) for (int i = 0; i < 5; i++) microThreads.create_thread(std::bind(&CScheduler::serviceQueue, µTasks)); - MicroSleep(600); + UninterruptibleSleep(std::chrono::microseconds{600}); now = boost::chrono::system_clock::now(); // More threads and more tasks: diff --git a/src/util/time.cpp b/src/util/time.cpp index 5c1182850e..f6d0dd4da6 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -11,7 +11,6 @@ #include #include -#include #include #include @@ -76,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; diff --git a/src/util/time.h b/src/util/time.h index 445ca3acb3..77de1e047d 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -38,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 T GetTime(); -- cgit v1.2.3