Merge #16117: util: Replace boost sleep with std sleep

fanquake · fanquake · commit 97aadf98d0b8 · 2020-03-06T15:41:00.000+08:00
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
diff --git 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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
                     });
         }
diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp
@@ -4,6 +4,7 @@
 
 #include <random.h>
 #include <scheduler.h>
+#include <util/time.h>
 
 #include <boost/thread.hpp>
 #include <boost/test/unit_test.hpp>
@@ -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, &microTasks));
 
-    MicroSleep(600);
+    UninterruptibleSleep(std::chrono::microseconds{600});
     now = boost::chrono::system_clock::now();
 
     // More threads and more tasks:
diff --git 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
@@ -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());
@@ -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>());
 }
diff --git 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/util/time.cpp b/src/util/time.cpp
@@ -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()
@@ -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;
diff --git a/src/util/time.h b/src/util/time.h
@@ -10,6 +10,8 @@
 #include <string>
 #include <chrono>
 
+void UninterruptibleSleep(const std::chrono::microseconds& n);
+
 /**
  * Helper to count the seconds of a duration.
  *
@@ -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();
diff --git 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});
     }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@`
`10`	`10`	`static void Sleep100ms(benchmark::State& state)`
`11`	`11`	`{`
`12`	`12`	`while (state.KeepRunning()) {`
`13`		`- MilliSleep(100);`
	`13`	`+ UninterruptibleSleep(std::chrono::milliseconds{100});`
`14`	`14`	`}`
`15`	`15`	`}`
`16`	`16`
Original file line number	Diff line number	Diff line change
`@@ -524,7 +524,7 @@ static int CommandLineRPC(int argc, char *argv[])`
`524`	`524`	`}`
`525`	`525`	`catch (const CConnectionFailed&) {`
`526`	`526`	`if (fWait)`
`527`		`- MilliSleep(1000);`
	`527`	`+ UninterruptibleSleep(std::chrono::milliseconds{1000});`
`528`	`528`	`else`
`529`	`529`	`throw;`
`530`	`530`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ static void WaitForShutdown(NodeContext& node)`
`29`	`29`	`{`
`30`	`30`	`while (!ShutdownRequested())`
`31`	`31`	`{`
`32`		`- MilliSleep(200);`
	`32`	`+ UninterruptibleSleep(std::chrono::milliseconds{200});`
`33`	`33`	`}`
`34`	`34`	`Interrupt(node);`
`35`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -169,7 +169,7 @@ UniValue stop(const JSONRPCRequest& jsonRequest)`
`169`	`169`	`// this reply will get back to the client.`
`170`	`170`	`StartShutdown();`
`171`	`171`	`if (jsonRequest.params[0].isNum()) {`
`172`		`- MilliSleep(jsonRequest.params[0].get_int());`
	`172`	`+ UninterruptibleSleep(std::chrono::milliseconds{jsonRequest.params[0].get_int()});`
`173`	`173`	`}`
`174`	`174`	`return PACKAGE_NAME " stopping";`
`175`	`175`	`}`
Original file line number	Diff line number	Diff line change
`@@ -138,7 +138,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)`
`138`	`138`	`int64_t time_start = GetTimeMillis();`
`139`	`139`	`while (!filter_index.BlockUntilSyncedToCurrentChain()) {`
`140`	`140`	`BOOST_REQUIRE(time_start + timeout_ms > GetTimeMillis());`
`141`		`- MilliSleep(100);`
	`141`	`+ UninterruptibleSleep(std::chrono::milliseconds{100});`
`142`	`142`	`}`
`143`	`143`
`144`	`144`	`// Check that filter index has all blocks that were in the chain before it started.`
Original file line number	Diff line number	Diff line change
`@@ -393,7 +393,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks)`
`393`	`393`	`CCheckQueueControl<FakeCheck> control(queue.get());`
`394`	`394`	`// While sleeping, no other thread should execute to this point`
`395`	`395`	`auto observed = ++nThreads;`
`396`		`- MilliSleep(10);`
	`396`	`+ UninterruptibleSleep(std::chrono::milliseconds{10});`
`397`	`397`	`fails += observed != nThreads;`
`398`	`398`	`});`
`399`	`399`	`}`