Merge #19189: refactor: Replace RecursiveMutex with Mutex in timedata.cpp

cc5c0d2299 refactor: Fix formatting of timedata.cpp (Hennadii Stepanov)
c2410ceb84 refactor: Replace RecursiveMutex with Mutex in timedata.cpp (Hennadii Stepanov)

Pull request description:

  Only `GetTimeOffset()` and `AddTimeData()` functions lock this mutex. They do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_timeoffset_mutex` could be a non-recursive mutex.

  Related to #19180.

ACKs for top commit:
  MarcoFalke:
    ACK cc5c0d2299 , checked the second commit with  --word-diff-regex=. --ignore-all-space -U0 🦉
  vasild:
    ACK cc5c0d22 verified that recursion is not happening

Tree-SHA512: 38f6df689374d4a1a0e9aedb3ed5e885d8285c4da6b75f9bc84ae036936a159ef8276462db33b4f4dd5c71c6312fa9b45380f7a5726959665bc71dc39031be88
This commit is contained in:
MarcoFalke 2020-06-08 07:03:36 -04:00
commit 374fd6fc8b
No known key found for this signature in database
GPG key ID: CE2B75697E69A548

View file

@ -15,9 +15,8 @@
#include <util/translation.h>
#include <warnings.h>
static RecursiveMutex cs_nTimeOffset;
static int64_t nTimeOffset GUARDED_BY(cs_nTimeOffset) = 0;
static Mutex g_timeoffset_mutex;
static int64_t nTimeOffset GUARDED_BY(g_timeoffset_mutex) = 0;
/**
* "Never go to sea with two chronometers; take one or three."
@ -28,7 +27,7 @@ static int64_t nTimeOffset GUARDED_BY(cs_nTimeOffset) = 0;
*/
int64_t GetTimeOffset()
{
LOCK(cs_nTimeOffset);
LOCK(g_timeoffset_mutex);
return nTimeOffset;
}
@ -46,7 +45,7 @@ static int64_t abs64(int64_t n)
void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
{
LOCK(cs_nTimeOffset);
LOCK(g_timeoffset_mutex);
// Ignore duplicates
static std::set<CNetAddr> setKnown;
if (setKnown.size() == BITCOIN_TIMEDATA_MAX_SAMPLES)
@ -57,7 +56,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
// Add data
static CMedianFilter<int64_t> vTimeOffsets(BITCOIN_TIMEDATA_MAX_SAMPLES, 0);
vTimeOffsets.input(nOffsetSample);
LogPrint(BCLog::NET,"added time data, samples %d, offset %+d (%+d minutes)\n", vTimeOffsets.size(), nOffsetSample, nOffsetSample/60);
LogPrint(BCLog::NET, "added time data, samples %d, offset %+d (%+d minutes)\n", vTimeOffsets.size(), nOffsetSample, nOffsetSample / 60);
// There is a known issue here (see issue #4521):
//
@ -76,30 +75,24 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
// So we should hold off on fixing this and clean it up as part of
// a timing cleanup that strengthens it in a number of other ways.
//
if (vTimeOffsets.size() >= 5 && vTimeOffsets.size() % 2 == 1)
{
if (vTimeOffsets.size() >= 5 && vTimeOffsets.size() % 2 == 1) {
int64_t nMedian = vTimeOffsets.median();
std::vector<int64_t> vSorted = vTimeOffsets.sorted();
// Only let other nodes change our time by so much
if (abs64(nMedian) <= std::max<int64_t>(0, gArgs.GetArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT)))
{
if (abs64(nMedian) <= std::max<int64_t>(0, gArgs.GetArg("-maxtimeadjustment", DEFAULT_MAX_TIME_ADJUSTMENT))) {
nTimeOffset = nMedian;
}
else
{
} else {
nTimeOffset = 0;
static bool fDone;
if (!fDone)
{
if (!fDone) {
// If nobody has a time different than ours but within 5 minutes of ours, give a warning
bool fMatch = false;
for (const int64_t nOffset : vSorted)
if (nOffset != 0 && abs64(nOffset) < 5 * 60)
fMatch = true;
for (const int64_t nOffset : vSorted) {
if (nOffset != 0 && abs64(nOffset) < 5 * 60) fMatch = true;
}
if (!fMatch)
{
if (!fMatch) {
fDone = true;
bilingual_str strMessage = strprintf(_("Please check that your computer's date and time are correct! If your clock is wrong, %s will not work properly."), PACKAGE_NAME);
SetMiscWarning(strMessage.translated);
@ -113,8 +106,7 @@ void AddTimeData(const CNetAddr& ip, int64_t nOffsetSample)
LogPrint(BCLog::NET, "%+d ", n); /* Continued */
}
LogPrint(BCLog::NET, "| "); /* Continued */
LogPrint(BCLog::NET, "nTimeOffset = %+d (%+d minutes)\n", nTimeOffset, nTimeOffset/60);
LogPrint(BCLog::NET, "nTimeOffset = %+d (%+d minutes)\n", nTimeOffset, nTimeOffset / 60);
}
}
}