This repository has been archived by the owner on Jan 22, 2025. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
There is a possible deadlock caused by double readlock in fn
get_transaction_status
.block_commitment_cache
is anArc<RwLock<...>>
.The first readlock is on L1434
solana/rpc/src/rpc.rs
Lines 1434 to 1436 in fbf7143
The second readlock is on L253 in fn
bank
solana/rpc/src/rpc.rs
Lines 250 to 254 in fbf7143
For more details on this kind of deadlock, see
https://www.reddit.com/r/rust/comments/urnqz8/different_behaviors_of_recursive_read_locks_in/
Summary of Changes
The fix is to move the first readlock after the calling of fn
bank
.But I wonder if this is correct or optimal.
optimistically_confirmed
should be protected byblock_commitment_cache
? ORr_block_commitment_cache
immediately after the creation ofconfirmations
on L1440-1448.Fixes #