core/filtermaps: fix deadlock in filtermap callback #31708
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Apr 25, 2025
zsfelfoldi
added a commit
that referenced
this pull request
May 2, 2025
This PR fixes the out-of-range block number logic of `getBlockLvPointer` which sometimes caused searches to fail if the head was updated in the wrong moment. This logic ensures that querying the pointer of a future block returns the pointer after the last fully indexed block (instead of failing) and therefore an async range update will not cause the search to fail. Earier this behaviour only worked when `headIndexed` was true and `headDelimiter` pointed to the end of the indexed range. Now it also works for an unfinished index. This logic is also moved from `FilterMaps.getBlockLvPointer` to `FilterMapsMatcherBackend.GetBlockLvPointer` because it is only required by the search anyways. `FilterMaps.getBlockLvPointer` now only returns a pointer for existing blocks, consistently with how it is used in the indexer/renderer. Note that this unhandled case has been present in the code for a long time but went unnoticed because either one of two previously fixed bugs did prevent it from being triggered; the incorrectly positive `tempRange.headIndexed` (fixed in #31680), though caused other problems, prevented this one from being triggered as with a positive `headIndexed` no database read was triggered in `getBlockLvPointer`. Also, the unnecessary `indexLock` in `synced()` (fixed in #31708) usually did prevent the search seeing the temp range and therefore avoided noticeable issues.
ucwong
pushed a commit
to CortexFoundation/CortexTheseus
that referenced
this pull request
May 7, 2025
This PR fixes the out-of-range block number logic of `getBlockLvPointer` which sometimes caused searches to fail if the head was updated in the wrong moment. This logic ensures that querying the pointer of a future block returns the pointer after the last fully indexed block (instead of failing) and therefore an async range update will not cause the search to fail. Earier this behaviour only worked when `headIndexed` was true and `headDelimiter` pointed to the end of the indexed range. Now it also works for an unfinished index. This logic is also moved from `FilterMaps.getBlockLvPointer` to `FilterMapsMatcherBackend.GetBlockLvPointer` because it is only required by the search anyways. `FilterMaps.getBlockLvPointer` now only returns a pointer for existing blocks, consistently with how it is used in the indexer/renderer. Note that this unhandled case has been present in the code for a long time but went unnoticed because either one of two previously fixed bugs did prevent it from being triggered; the incorrectly positive `tempRange.headIndexed` (fixed in ethereum/go-ethereum#31680), though caused other problems, prevented this one from being triggered as with a positive `headIndexed` no database read was triggered in `getBlockLvPointer`. Also, the unnecessary `indexLock` in `synced()` (fixed in ethereum/go-ethereum#31708) usually did prevent the search seeing the temp range and therefore avoided noticeable issues.
0g-wh
pushed a commit
to 0g-wh/0g-geth
that referenced
this pull request
May 8, 2025
This PR fixes a deadlock situation is deleteTailEpoch that might arise when range delete is running in iterator based fallback mode (either using leveldb database or the hashdb state storage scheme). In this case a stopCb callback is called periodically that does check events, including matcher sync requests, in which case it tries to acquire indexLock for read access, while deleteTailEpoch already held it for write access. This pull request removes the indexLock acquiring in `FilterMapsMatcherBackend.synced` as this function is only called in the indexLoop. Fixes ethereum#31700
0g-wh
pushed a commit
to 0g-wh/0g-geth
that referenced
this pull request
May 8, 2025
This PR fixes the out-of-range block number logic of `getBlockLvPointer` which sometimes caused searches to fail if the head was updated in the wrong moment. This logic ensures that querying the pointer of a future block returns the pointer after the last fully indexed block (instead of failing) and therefore an async range update will not cause the search to fail. Earier this behaviour only worked when `headIndexed` was true and `headDelimiter` pointed to the end of the indexed range. Now it also works for an unfinished index. This logic is also moved from `FilterMaps.getBlockLvPointer` to `FilterMapsMatcherBackend.GetBlockLvPointer` because it is only required by the search anyways. `FilterMaps.getBlockLvPointer` now only returns a pointer for existing blocks, consistently with how it is used in the indexer/renderer. Note that this unhandled case has been present in the code for a long time but went unnoticed because either one of two previously fixed bugs did prevent it from being triggered; the incorrectly positive `tempRange.headIndexed` (fixed in ethereum#31680), though caused other problems, prevented this one from being triggered as with a positive `headIndexed` no database read was triggered in `getBlockLvPointer`. Also, the unnecessary `indexLock` in `synced()` (fixed in ethereum#31708) usually did prevent the search seeing the temp range and therefore avoided noticeable issues.
Rampex1
pushed a commit
to streamingfast/go-ethereum
that referenced
this pull request
May 15, 2025
This PR fixes a deadlock situation is deleteTailEpoch that might arise when range delete is running in iterator based fallback mode (either using leveldb database or the hashdb state storage scheme). In this case a stopCb callback is called periodically that does check events, including matcher sync requests, in which case it tries to acquire indexLock for read access, while deleteTailEpoch already held it for write access. This pull request removes the indexLock acquiring in `FilterMapsMatcherBackend.synced` as this function is only called in the indexLoop. Fixes ethereum#31700
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR fixes a deadlock situation is deleteTailEpoch that might arise when
range delete is running in iterator based fallback mode (either using leveldb
database or the hashdb state storage scheme).
In this case a stopCb callback is called periodically that does check events,
including matcher sync requests, in which case it tries to acquire indexLock
for read access, while deleteTailEpoch already held it for write access.
This pull request removes the indexLock acquiring in
FilterMapsMatcherBackend.syncedas this function is only called in the indexLoop.
Fixes #31700