Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve shared data locking and Synchronisation #256

Closed
Naviabheeman opened this issue May 9, 2023 · 3 comments
Closed

Improve shared data locking and Synchronisation #256

Naviabheeman opened this issue May 9, 2023 · 3 comments

Comments

@Naviabheeman
Copy link
Contributor

In the bitcoin-core codebase, many improvements have been made to keep shared data protected by critical section.

These classes are some examples:

  • NetEventsInterface
  • CNodeState
  • CConnman
  • CMainSignals
  • CBlockPolicyEstimator
  • CWallet
  • CCheckQueue

GUARDED_BY, EXCLUSIVE_LOCKS_REQUIRED and SHARED_LOCKS_REQUIRED definitions are available in Tapyrus. These can be used to make data thread safe

@Naviabheeman
Copy link
Contributor Author

check.bitcoin 25077

@Naviabheeman
Copy link
Contributor Author

Naviabheeman commented Aug 4, 2023

CCheckQueue and CMainSignals were updated as part of boost::thread to std::thread migration.

CWallet and CBlockPolicyEstimator classes in Bitcoin are very different from our classes. So adding GUARDED_BY and EXCLUSIVE_LOCKS_REQUIRED caused deadlock.

CNodeState and CConnman classes are further divided in bitcoin and new mutexex added to protect the new data. Our classes are sufficiently protected. So these do not need changes.

NetEventsInterface and CAddrMan classes have exactly the same mutex as bitcoin. but adding more locks to match bitcoin code caused deadlock in functional tests. SO these are not changed.

Our old class names are changed to Mutex and RecursiveMutex like bitcoin. RecursiveMutex may be locked more than once by the same thread without blocking. Mutex is always blocking.

Most of our mutexes like cs_main, cs_wallet, mempool.cs are recursive. Many are global. These are locked at the beginning of a message processing and not just at the places where shared data is accessed or modified. It would require a huge effort to redesign these and make them non recursive. Performance benefit would be significant. But development and testing effort need to be planned before attempting it.

@azuchi
Copy link
Contributor

azuchi commented May 16, 2024

Complete by #276

@azuchi azuchi closed this as completed May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants