Skip to content

Conversation

jonatack
Copy link
Member

@jonatack jonatack commented Jan 7, 2022

New helper function BlockManager::WriteBlockIndexDB() added in #23974 has a thread safety lock annotation in its declaration but is missing the corresponding run-time lock assertion in its definition.

Per doc/developer-notes.md: "Combine annotations in function declarations with run-time asserts in function definitions."

The new helper function, BlockManager::WriteBlockIndexDB(),
has a thread safety lock annotation in its declaration but is
missing the corresponding run-time lock assertion in its definition.

Per doc/developer-notes.md: "Combine annotations in function
declarations with run-time asserts in function definitions."
@fanquake
Copy link
Member

fanquake commented Jan 7, 2022

Combine into #22932 once it's rebased?

@jonatack
Copy link
Member Author

jonatack commented Jan 7, 2022

Combine into #22932 once it's rebased?

This code doesn't touch that pull. (It's also been open for several months without ACKs, so I'm planning to split it up into smaller pulls to ease review.)

@fanquake
Copy link
Member

fanquake commented Jan 7, 2022

This code doesn't touch that pull.

That would be difficult given the code modified here has only been added to the repo since that PR was last rebased. I suggested combining because #22932 is full of changes very similar to this one.

@jonatack
Copy link
Member Author

jonatack commented Jan 7, 2022

#22932 does not touch this function and there is no guarantee that it will be merged. I plan to reduce its scope or perhaps close it.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 8, 2022

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #22564 ([WIP] refactor: Move mutable globals cleared in ::UnloadBlockIndex to BlockManager by dongcarl)

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.

@maflcko
Copy link
Member

maflcko commented Jan 8, 2022

cr ACK 1823766

@maflcko maflcko merged commit 6182e50 into bitcoin:master Jan 8, 2022
@jonatack jonatack deleted the add-thread-safety-lock-assertion-to-WriteBlockIndexDB branch January 8, 2022 08:39
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 9, 2022
…iteBlockIndexDB()

1823766 refactor: add thread safety lock assertion to WriteBlockIndexDB() (Jon Atack)

Pull request description:

  New helper function `BlockManager::WriteBlockIndexDB()` added in bitcoin#23974 has a thread safety lock annotation in its declaration but is missing the corresponding run-time lock assertion in its definition.

  Per doc/developer-notes.md: "Combine annotations in function declarations with run-time asserts in function definitions."

ACKs for top commit:
  MarcoFalke:
    cr ACK 1823766

Tree-SHA512: b915e6b105c38b8bbe04ad810aefa68e940a13b8dd265e79563a2aaefc93ffa031d56a7f3c481a5ada90de7c2ddd3b419dcfa46c22fa26c22f95eda15cd243bc
@bitcoin bitcoin locked and limited conversation to collaborators Jan 8, 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.

4 participants