Page MenuHomePhabricator

Fix lock in getVerificationProgress
AbandonedPublic

Authored by jasonbcox on Mon, Nov 25, 20:10.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

This fixes our sanitizer builds which are currently red on master.

This is related to the currently open Core issue here: https://github.com/bitcoin/bitcoin/issues/15994
and associated un-merged PR mentioned in the comment in GuessVerificationProgress:

// This function assumes the lock on cs_main is already held (see the
// above comment). This is a temporary check until PR15997 is backported.
// https://github.com/bitcoin/bitcoin/pull/15997
AssertLockHeld(cs_main);

However, PR15997 is currently closed, pending a different approach being reviewed in https://github.com/bitcoin/bitcoin/pull/16426

Since it's unlikely that we'll be pulling in a refactor that large any time soon, we can apply this patch to fix the build in the mean time.

Test Plan

ninja
Run TSAN, ASAN, UBSAN builds on TeamCity

Diff Detail

Repository
rABC Bitcoin ABC
Branch
fixlock
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8279
Build 14583: Bitcoin ABC Buildbot
Build 14582: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Mon, Nov 25, 20:10
jasonbcox planned changes to this revision.Mon, Nov 25, 20:10
jasonbcox edited the summary of this revision. (Show Details)Mon, Nov 25, 20:56
jasonbcox edited the test plan for this revision. (Show Details)
jasonbcox edited the summary of this revision. (Show Details)
jasonbcox requested review of this revision.Mon, Nov 25, 21:12
deadalnix requested changes to this revision.Mon, Nov 25, 22:17

As The GuessVerificationProgress doc says:

//! Guess how far we are in the verification process at the given block index
//! require cs_main if pindex has not been validated yet (because nChainTx might
//! be unset)

So this does not seems to be an appropriate measure. Because no description of the problem is provided, it is difficult to suggest any alternative.

This revision now requires changes to proceed.Mon, Nov 25, 22:17