Anyone willing to review? :-)

@ajtowns @TheBlueMatt Thanks for reviewing! I've now addressed your feedback. Would you mind re-reviewing? :-)

General note: Instead of documenting the locking assumptions in the OP of this pull request, wouldn't it be better to put them in the header files? Imo, this increases review efficiency.

@MarcoFalke You mean as an interim before we've added GUARDED_BY(…) annotations for each guarded variable?

No, I meant adding the GUARDED_BY to header files whenever possible

@MarcoFalke Sure! New version with GUARDED_BY + the implied EXCLUSIVE_LOCKS_REQUIRED :-)

Could you please review? :-)

Hrmm, the i686-w64-mingw32 build seems to timeout for some reason.

Hrmm, the i686-w64-mingw32 build seems to timeout for some reason.

Should be fixed by rebasing after #12772

Rebased!

9bd9d9f8302d4cdde2f25f4d7a008f8f0a0ccea2 looks good to me, fwiw, but it needs another rebase anyway to cope with scoped enums.

Rebased!

@ajtowns Thanks for the review. Please re-review :-)

Please just put a cs_main at the top of the load-from disk loop thing, there is no reason to lock and unlock cs_main 20 times during single-threaded init.

Looks like you never did this.

@TheBlueMatt Can you confirm that your suggestion is addressed in the latest version? :-) If not I might need help identifying the "load-from disk loop thing" :-)

@TheBlueMatt Friendly ping :-)

@MarcoFalke I'll do that once my other two open locking PRs #13128 and #13123 have been processed (either merged or closed). I try to limit my work in progress in the form of open pull requests :-)

I think this can be reopened and rebased.

@MarcoFalke I force-pushed before re-opening, so now I'm unable to re-open. Could you re-open? :-)

I can't force push 7e11716932b8a1f6f1aa2c06e1990b64950a89bc to your branch either, since the pull request is closed.

Try

git push origin 7e11716932b8a1f6f1aa2c06e1990b64950a89bc:init-and-validation-locks -f
# reopen
git push origin init-and-validation-locks -f

@MarcoFalke Thanks! Now re-opened and force-pushed. WIP for now.

@MarcoFalke I'm a bit unsure how to resolve the locking errors below (see Travis OS X output). Do you have any suggestion?

If m_wallet.chain().lock(…) succeeds then we have acquired cs_main and access to chainActive should be OK?

interfaces/wallet.cpp:335:22: error: reading variable 'chainActive' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
        num_blocks = ::chainActive.Height();
                     ^
interfaces/wallet.cpp:349:26: error: reading variable 'chainActive' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
            num_blocks = ::chainActive.Height();
                         ^
interfaces/wallet.cpp:380:22: error: reading variable 'chainActive' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
        num_blocks = ::chainActive.Height();
                     ^

The compiler doesn't understand that, though. You'd have to add a temporary annotation or use the interface (c.f. #14711)

re: #11652 (comment)

I'm a bit unsure how to resolve the locking errors below (see Travis OS X output). Do you have any suggestion?

You could use the LockAnnotation class to resolve these errors. See 081accb from #14437 for examples. Rebasing on top of #14711 would also resolve them.

@MarcoFalke Would you mind taking over this PR if it put it up for grabs? At the moment I'm afraid I don't have time to resolve the remaining issues.

At the moment I'm afraid I don't have time to resolve the remaining issues.

Are there other issues besides the need for lock annotations in the wallet? If not, we could just treat this PR as blocked until #14711 is merged, since it removes the need for them.

Closing in favour of PR #15191 and #15192. They cover everything in this PR except the chainActive locks.

I'll revisit the missing cs_main locks required for chainActive once #14711 is merged.