Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31124: util: Remove RandAddSeedPerfmon
Browse files Browse the repository at this point in the history
9bb92c0 util: Remove RandAddSeedPerfmon (Hodlinator)

Pull request description:

  `RegQueryValueExA(HKEY_PERFORMANCE_DATA, ...)` sometimes hangs *bitcoind.exe* on Windows during startup, at least on CI.

  We have other sources of entropy to seed randomness with on Windows, so should be alright removing this. Might resurrect if less drastic fix is found.

  Hopefully sufficient to fix #30390.

  CI debugged with temporary Windows stack trace dumping + Symbols in #30956.

ACKs for top commit:
  achow101:
    ACK 9bb92c0
  fanquake:
    ACK 9bb92c0
  hebasto:
    ACK 9bb92c0, I have reviewed the code and it looks OK.
  laanwj:
    Code review ACK  9bb92c0

Tree-SHA512: d3f26b4dd0519ef957f23abaffc6be1fed339eae756aed18042422fc6f0bba4e8fa9a44bf903e54f72747e2d0108146c18fd80576d95fc20690a2daf9c83689d
  • Loading branch information
achow101 committed Oct 24, 2024
2 parents 7640cfd + 9bb92c0 commit 947f292
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 45 deletions.
4 changes: 2 additions & 2 deletions src/random.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ void SeedPeriodic(CSHA512& hasher, RNGState& rng) noexcept
// Add the events hasher into the mix
rng.SeedEvents(hasher);

// Dynamic environment data (performance monitoring, ...)
// Dynamic environment data (clocks, resource usage, ...)
auto old_size = hasher.Size();
RandAddDynamicEnv(hasher);
LogDebug(BCLog::RAND, "Feeding %i bytes of dynamic environment data into RNG\n", hasher.Size() - old_size);
Expand All @@ -616,7 +616,7 @@ void SeedStartup(CSHA512& hasher, RNGState& rng) noexcept
// Everything that the 'slow' seeder includes.
SeedSlow(hasher, rng);

// Dynamic environment data (performance monitoring, ...)
// Dynamic environment data (clocks, resource usage, ...)
auto old_size = hasher.Size();
RandAddDynamicEnv(hasher);

Expand Down
2 changes: 1 addition & 1 deletion src/random.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
*
* - RandAddPeriodic() seeds everything that fast seeding includes, but additionally:
* - A high-precision timestamp
* - Dynamic environment data (performance monitoring, ...)
* - Dynamic environment data (clocks, resource usage, ...)
* - Strengthen the entropy for 10 ms using repeated SHA512.
* This is run once every minute.
*
Expand Down
42 changes: 0 additions & 42 deletions src/randomenv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@

#ifdef WIN32
#include <windows.h>
#include <winreg.h>
#else
#include <fcntl.h>
#include <netinet/in.h>
Expand Down Expand Up @@ -64,45 +63,6 @@ extern char** environ; // NOLINT(readability-redundant-declaration): Necessary o

namespace {

void RandAddSeedPerfmon(CSHA512& hasher)
{
#ifdef WIN32
// Seed with the entire set of perfmon data

// This can take up to 2 seconds, so only do it every 10 minutes.
// Initialize last_perfmon to 0 seconds, we don't skip the first call.
static std::atomic<SteadyClock::time_point> last_perfmon{SteadyClock::time_point{0s}};
auto last_time = last_perfmon.load();
auto current_time = SteadyClock::now();
if (current_time < last_time + 10min) return;
last_perfmon = current_time;

std::vector<unsigned char> vData(250000, 0);
long ret = 0;
unsigned long nSize = 0;
const size_t nMaxSize = 10000000; // Bail out at more than 10MB of performance data
while (true) {
nSize = vData.size();
ret = RegQueryValueExA(HKEY_PERFORMANCE_DATA, "Global", nullptr, nullptr, vData.data(), &nSize);
if (ret != ERROR_MORE_DATA || vData.size() >= nMaxSize)
break;
vData.resize(std::min((vData.size() * 3) / 2, nMaxSize)); // Grow size of buffer exponentially
}
RegCloseKey(HKEY_PERFORMANCE_DATA);
if (ret == ERROR_SUCCESS) {
hasher.Write(vData.data(), nSize);
memory_cleanse(vData.data(), nSize);
} else {
// Performance data is only a best-effort attempt at improving the
// situation when the OS randomness (and other sources) aren't
// adequate. As a result, failure to read it is isn't considered critical,
// so we don't call RandFailure().
// TODO: Add logging when the logger is made functional before global
// constructors have been invoked.
}
#endif
}

/** Helper to easily feed data into a CSHA512.
*
* Note that this does not serialize the passed object (like stream.h's << operators do).
Expand Down Expand Up @@ -227,8 +187,6 @@ void AddAllCPUID(CSHA512& hasher)

void RandAddDynamicEnv(CSHA512& hasher)
{
RandAddSeedPerfmon(hasher);

// Various clocks
#ifdef WIN32
FILETIME ftime;
Expand Down

0 comments on commit 947f292

Please sign in to comment.