Skip to content

Commit c748fd8

Browse files
Recover from panic in HandleTransaction to prevent peer crash (#5472) (#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>
1 parent 7d8bed5 commit c748fd8

2 files changed

Lines changed: 67 additions & 6 deletions

File tree

core/chaincode/handler.go

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,41 @@ type handleFunc func(*pb.ChaincodeMessage, *TransactionContext) (*pb.ChaincodeMe
227227
// returned by the delegate are sent to the chat stream. Any errors returned by the
228228
// delegate are packaged as chaincode error messages.
229229
func (h *Handler) HandleTransaction(msg *pb.ChaincodeMessage, delegate handleFunc) {
230+
startTime := time.Now()
231+
meterLabels := []string{
232+
"type", msg.Type.String(),
233+
"channel", msg.ChannelId,
234+
"chaincode", h.chaincodeID,
235+
}
236+
237+
// Recover from panics that can occur when the transaction context is cleaned up
238+
// (e.g., iterators closed) due to execution timeout while this goroutine is still
239+
// actively using those resources. This prevents peer crashes from nil pointer
240+
// dereferences in the underlying LevelDB iterator.
241+
// See https://github.com/hyperledger/fabric/issues/5048
242+
defer func() {
243+
if r := recover(); r != nil {
244+
chaincodeLogger.Errorf("[%s] Recovered from panic handling %s: %v", shorttxid(msg.Txid), msg.Type, r)
245+
resp := &pb.ChaincodeMessage{
246+
Type: pb.ChaincodeMessage_ERROR,
247+
Payload: []byte(fmt.Sprintf("%s failed: transaction ID: %s: panic during execution", msg.Type, msg.Txid)),
248+
Txid: msg.Txid,
249+
ChannelId: msg.ChannelId,
250+
}
251+
h.ActiveTransactions.Remove(msg.ChannelId, msg.Txid)
252+
h.serialSendAsync(resp)
253+
254+
meterLabels = append(meterLabels, "success", "false")
255+
h.Metrics.ShimRequestDuration.With(meterLabels...).Observe(time.Since(startTime).Seconds())
256+
h.Metrics.ShimRequestsCompleted.With(meterLabels...).Add(1)
257+
}
258+
}()
259+
230260
chaincodeLogger.Debugf("[%s] handling %s from chaincode", shorttxid(msg.Txid), msg.Type.String())
231261
if !h.registerTxid(msg) {
232262
return
233263
}
234264

235-
startTime := time.Now()
236265
var txContext *TransactionContext
237266
var err error
238267
if msg.Type == pb.ChaincodeMessage_INVOKE_CHAINCODE {
@@ -241,11 +270,6 @@ func (h *Handler) HandleTransaction(msg *pb.ChaincodeMessage, delegate handleFun
241270
txContext, err = h.isValidTxSim(msg.ChannelId, msg.Txid, "no ledger context")
242271
}
243272

244-
meterLabels := []string{
245-
"type", msg.Type.String(),
246-
"channel", msg.ChannelId,
247-
"chaincode", h.chaincodeID,
248-
}
249273
h.Metrics.ShimRequestsReceived.With(meterLabels...).Add(1)
250274

251275
var resp *pb.ChaincodeMessage

core/chaincode/handler_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,43 @@ var _ = Describe("Handler", func() {
490490
}))
491491
})
492492
})
493+
494+
// Regression test for https://github.com/hyperledger/fabric/issues/5048
495+
// When a transaction times out during a range query, the timeout path closes
496+
// the LevelDB iterator while the handler goroutine is still using it, causing
497+
// a nil pointer dereference panic. The recover() in HandleTransaction prevents
498+
// this from crashing the peer.
499+
Context("when the delegate panics", func() {
500+
It("recovers and sends an error response", func() {
501+
panickingDelegate := func(msg *pb.ChaincodeMessage, txContext *chaincode.TransactionContext) (*pb.ChaincodeMessage, error) {
502+
panic("simulated nil pointer dereference from closed iterator")
503+
}
504+
505+
Expect(func() {
506+
handler.HandleTransaction(incomingMessage, panickingDelegate)
507+
}).NotTo(Panic())
508+
509+
Eventually(fakeChatStream.SendCallCount).Should(Equal(1))
510+
msg := fakeChatStream.SendArgsForCall(0)
511+
Expect(msg.Type).To(Equal(pb.ChaincodeMessage_ERROR))
512+
Expect(msg.Txid).To(Equal("tx-id"))
513+
Expect(msg.ChannelId).To(Equal("channel-id"))
514+
Expect(string(msg.Payload)).To(ContainSubstring("panic during execution"))
515+
})
516+
517+
It("deregisters the transaction ID", func() {
518+
panickingDelegate := func(msg *pb.ChaincodeMessage, txContext *chaincode.TransactionContext) (*pb.ChaincodeMessage, error) {
519+
panic("simulated panic")
520+
}
521+
522+
handler.HandleTransaction(incomingMessage, panickingDelegate)
523+
524+
Expect(fakeTransactionRegistry.RemoveCallCount()).To(Equal(1))
525+
channelID, transactionID := fakeTransactionRegistry.RemoveArgsForCall(0)
526+
Expect(channelID).To(Equal("channel-id"))
527+
Expect(transactionID).To(Equal("tx-id"))
528+
})
529+
})
493530
})
494531

495532
Describe("HandlePutState", func() {

0 commit comments

Comments
 (0)