Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented May 16, 2022

In non-test/benchmarking code, there are three cases of the NO_THREAD_SAFETY_ANALYSIS annotation which are accompanied with TODO comments.

This PR adds proper thread safety annotations instead of NO_THREAD_SAFETY_ANALYSIS.

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23507 (Refactor: Improve API design of ScriptToUniv, TxToUniv etc to return the UniValue instead of mutating a parameter/reference by mjdietzx)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@jonatack
Copy link
Member

Concept ACK, good idea.

@laanwj
Copy link
Member

laanwj commented May 18, 2022

LGTM
Code review ACK a55db4e

@maflcko maflcko merged commit 629e250 into bitcoin:master May 18, 2022
@w0xlt
Copy link
Contributor

w0xlt commented May 18, 2022

Post-merge ACK a55db4e

@hebasto hebasto deleted the 220516-todo branch May 18, 2022 16:51
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
…m non-test/benchmarking code

a55db4e Add more proper thread safety annotations (Hennadii Stepanov)
8cfe93e Add proper thread safety annotation to `CWallet::GetTxConflicts()` (Hennadii Stepanov)
ca446f2 Add proper thread safety annotation to `CachedTxGetAvailableCredit()` (Hennadii Stepanov)

Pull request description:

  In non-test/benchmarking code, there are three cases of the `NO_THREAD_SAFETY_ANALYSIS` annotation which are accompanied with `TODO` comments.

  This PR adds proper thread safety annotations instead of `NO_THREAD_SAFETY_ANALYSIS`.

ACKs for top commit:
  laanwj:
    Code review ACK a55db4e

Tree-SHA512: 806d72eebc1edf088bfa435c8cd11465be0de6789798dd92abd008425516768acb864a73d834a49d412bb10f7fccfb47473f998cb72739dab6caeef6bcfaf191
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 8, 2022
Summary:
This is a backport of [[bitcoin/bitcoin#25148 | core#25148]]

Depends on D12784

Notes:
 - there is one more occurence of NO_THREAD_SAFETY_ANALYSIS in avalanche/processor.cpp
 - `CWallet::IsSpent` and `CWallet::ReacceptWalletTransaction` already had the annotation, so I also added the assertion (this is still not done on Core's master branch)

Test Plan:
With Debug and TSAN

`ninja all check check-functional`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D12809
@bitcoin bitcoin locked and limited conversation to collaborators May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants