Skip to content

Commit 6a67397

Browse files
authored
Recover from panic in HandleTransaction to prevent peer crash (#5472)
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 Signed-off-by: Jaskirat-s7 <jaskiratsingh7812@gmail.com>
1 parent da3c2a8 commit 6a67397

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
@@ -245,12 +245,41 @@ type handleFunc func(*pb.ChaincodeMessage, *TransactionContext) (*pb.ChaincodeMe
245245
// returned by the delegate are sent to the chat stream. Any errors returned by the
246246
// delegate are packaged as chaincode error messages.
247247
func (h *Handler) HandleTransaction(msg *pb.ChaincodeMessage, delegate handleFunc) {
248+
startTime := time.Now()
249+
meterLabels := []string{
250+
"type", msg.Type.String(),
251+
"channel", msg.ChannelId,
252+
"chaincode", h.chaincodeID,
253+
}
254+
255+
// Recover from panics that can occur when the transaction context is cleaned up
256+
// (e.g., iterators closed) due to execution timeout while this goroutine is still
257+
// actively using those resources. This prevents peer crashes from nil pointer
258+
// dereferences in the underlying LevelDB iterator.
259+
// See https://github.com/hyperledger/fabric/issues/5048
260+
defer func() {
261+
if r := recover(); r != nil {
262+
chaincodeLogger.Errorf("[%s] Recovered from panic handling %s: %v", shorttxid(msg.Txid), msg.Type, r)
263+
resp := &pb.ChaincodeMessage{
264+
Type: pb.ChaincodeMessage_ERROR,
265+
Payload: []byte(fmt.Sprintf("%s failed: transaction ID: %s: panic during execution", msg.Type, msg.Txid)),
266+
Txid: msg.Txid,
267+
ChannelId: msg.ChannelId,
268+
}
269+
h.ActiveTransactions.Remove(msg.ChannelId, msg.Txid)
270+
h.serialSendAsync(resp)
271+
272+
meterLabels = append(meterLabels, "success", "false")
273+
h.Metrics.ShimRequestDuration.With(meterLabels...).Observe(time.Since(startTime).Seconds())
274+
h.Metrics.ShimRequestsCompleted.With(meterLabels...).Add(1)
275+
}
276+
}()
277+
248278
chaincodeLogger.Debugf("[%s] handling %s from chaincode", shorttxid(msg.Txid), msg.Type.String())
249279
if !h.registerTxid(msg) {
250280
return
251281
}
252282

253-
startTime := time.Now()
254283
var txContext *TransactionContext
255284
var err error
256285
if msg.Type == pb.ChaincodeMessage_INVOKE_CHAINCODE {
@@ -259,11 +288,6 @@ func (h *Handler) HandleTransaction(msg *pb.ChaincodeMessage, delegate handleFun
259288
txContext, err = h.isValidTxSim(msg.ChannelId, msg.Txid, "no ledger context")
260289
}
261290

262-
meterLabels := []string{
263-
"type", msg.Type.String(),
264-
"channel", msg.ChannelId,
265-
"chaincode", h.chaincodeID,
266-
}
267291
h.Metrics.ShimRequestsReceived.With(meterLabels...).Add(1)
268292

269293
var resp *pb.ChaincodeMessage

core/chaincode/handler_test.go

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

500537
Describe("HandlePutState", func() {

0 commit comments

Comments
 (0)