Recover from panic in HandleTransaction to prevent peer crash#5472
Merged
pfi79 merged 1 commit intoMay 6, 2026
Merged
Conversation
Jaskirat-s7
force-pushed
the
fix/recover-panic-on-iterator-close-5048
branch
from
f5f6a65 to
1c22fe3
Compare
pfi79
reviewed
May 6, 2026
pfi79
reviewed
May 6, 2026
Jaskirat-s7
force-pushed
the
fix/recover-panic-on-iterator-close-5048
branch
from
1c22fe3 to
276964a
Compare
pfi79
reviewed
May 6, 2026
When a transaction times out during a range query, the timeout path closes LevelDB iterators while the handler goroutine is still using them, causing a nil pointer dereference that crashes the peer. Add a deferred recover() in HandleTransaction to catch such panics and return an error response instead of crashing. This is safe because the transaction has already timed out and the resources are being freed. The panic value is logged but not included in the response payload to avoid violating endorsement determinism across peers. Metrics are recorded in the recover path to match the normal code path. Also adds regression tests that verify the panic is recovered and an error response is sent. Fixes hyperledger#5048 Signed-off-by: Jaskirat-s7 <jaskiratsingh7812@gmail.com>
Jaskirat-s7
force-pushed
the
fix/recover-panic-on-iterator-close-5048
branch
from
276964a to
baad0b4
Compare
pfi79
approved these changes
May 6, 2026
pfi79
approved these changes
May 6, 2026
Contributor
|
@Mergifyio backport release-2.5 |
✅ Backports have been createdDetails
|
pfi79
pushed a commit
that referenced
this pull request
May 7, 2026
…#5473) When a transaction times out during a range query, the timeout path closes LevelDB iterators while the handler goroutine is still using them, causing a nil pointer dereference that crashes the peer. Add a deferred recover() in HandleTransaction to catch such panics and return an error response instead of crashing. This is safe because the transaction has already timed out and the resources are being freed. The panic value is logged but not included in the response payload to avoid violating endorsement determinism across peers. Metrics are recorded in the recover path to match the normal code path. Also adds regression tests that verify the panic is recovered and an error response is sent. Fixes #5048 (cherry picked from commit 6a67397) Signed-off-by: Jaskirat-s7 <jaskiratsingh7812@gmail.com> Co-authored-by: Jaskirat Singh <jaskiratsingh7812@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
When a transaction times out during a range query, the timeout path in
Execute()closes LevelDB iterators viaCloseQueryIterators()while the handler goroutine (spawned byhandleMessageReadyState) is still actively callingiter.Next(). This causes a nil pointer dereference in the LevelDB iterator that crashes the peer process.Changes
recover()inHandleTransaction(core/chaincode/handler.go) to catch panics from concurrent iterator access and return an error response instead of crashing the peerTesting
All 320 existing tests pass, including the 2 new regression tests: