Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jun 8, 2020

Another step on the way to replacing all of the RecursiveMutex instances with the Mutex ones.

This PR removes the RecursiveMutex object by splitting it into two Mutex objects, and ensuring they are always acquired in the same order.

Related to #19303.

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f7f41a0

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 16, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

@hebasto hebasto marked this pull request as draft October 6, 2020 19:54
@hebasto hebasto marked this pull request as ready for review November 8, 2020 20:52
@hebasto hebasto changed the title refactor: Replace RecursiveMutex with Mutex in Get{Data,Blocks}Dir() util: Get rid of RecursiveMutex in Get{Blocks,Data}Dir Nov 8, 2020
@hebasto
Copy link
Member Author

hebasto commented Nov 8, 2020

Reworked without introducing a new helper function.

ping @vasild @promag @MarcoFalke

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

The commit message reads just

util: Get rid of RecursiveMutex in Get{Blocks,Data}Dir

maybe it would be good to explain that we remove the recursive mutex by splitting it to two mutexes and ensuring they are always acquired in the same order.

This change removes the RecursiveMutex object by splitting it into two
Mutex objects, and ensuring they are always acquired in the same order.
-BEGIN VERIFY SCRIPT-
sed -i 's/pathCachedNetSpecific/g_datadir_path_cached_net_specific/' src/util/system.cpp
sed -i 's/pathCached/g_datadir_path_cached/' src/util/system.cpp
-END VERIFY SCRIPT-
@hebasto
Copy link
Member Author

hebasto commented Nov 9, 2020

Updated 0173e68 -> 7b90f77 (pr19213.02 -> pr19213.03, diff):

Looks good.

The commit message reads just

util: Get rid of RecursiveMutex in Get{Blocks,Data}Dir

maybe it would be good to explain that we remove the recursive mutex by splitting it to two mutexes and ensuring they are always acquired in the same order.

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 7b90f77

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@hebasto hebasto closed this Apr 20, 2021
@vasild
Copy link
Contributor

vasild commented Apr 20, 2021

Why close?

@hebasto
Copy link
Member Author

hebasto commented Apr 20, 2021

Why close?

It is incompatible with #21244.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
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.

3 participants