diff --git a/core/chaincode/handler.go b/core/chaincode/handler.go index 094d76d3f44..2307434a9e0 100644 --- a/core/chaincode/handler.go +++ b/core/chaincode/handler.go @@ -227,12 +227,41 @@ type handleFunc func(*pb.ChaincodeMessage, *TransactionContext) (*pb.ChaincodeMe // returned by the delegate are sent to the chat stream. Any errors returned by the // delegate are packaged as chaincode error messages. func (h *Handler) HandleTransaction(msg *pb.ChaincodeMessage, delegate handleFunc) { + startTime := time.Now() + meterLabels := []string{ + "type", msg.Type.String(), + "channel", msg.ChannelId, + "chaincode", h.chaincodeID, + } + + // Recover from panics that can occur when the transaction context is cleaned up + // (e.g., iterators closed) due to execution timeout while this goroutine is still + // actively using those resources. This prevents peer crashes from nil pointer + // dereferences in the underlying LevelDB iterator. + // See https://github.com/hyperledger/fabric/issues/5048 + defer func() { + if r := recover(); r != nil { + chaincodeLogger.Errorf("[%s] Recovered from panic handling %s: %v", shorttxid(msg.Txid), msg.Type, r) + resp := &pb.ChaincodeMessage{ + Type: pb.ChaincodeMessage_ERROR, + Payload: []byte(fmt.Sprintf("%s failed: transaction ID: %s: panic during execution", msg.Type, msg.Txid)), + Txid: msg.Txid, + ChannelId: msg.ChannelId, + } + h.ActiveTransactions.Remove(msg.ChannelId, msg.Txid) + h.serialSendAsync(resp) + + meterLabels = append(meterLabels, "success", "false") + h.Metrics.ShimRequestDuration.With(meterLabels...).Observe(time.Since(startTime).Seconds()) + h.Metrics.ShimRequestsCompleted.With(meterLabels...).Add(1) + } + }() + chaincodeLogger.Debugf("[%s] handling %s from chaincode", shorttxid(msg.Txid), msg.Type.String()) if !h.registerTxid(msg) { return } - startTime := time.Now() var txContext *TransactionContext var err error if msg.Type == pb.ChaincodeMessage_INVOKE_CHAINCODE { @@ -241,11 +270,6 @@ func (h *Handler) HandleTransaction(msg *pb.ChaincodeMessage, delegate handleFun txContext, err = h.isValidTxSim(msg.ChannelId, msg.Txid, "no ledger context") } - meterLabels := []string{ - "type", msg.Type.String(), - "channel", msg.ChannelId, - "chaincode", h.chaincodeID, - } h.Metrics.ShimRequestsReceived.With(meterLabels...).Add(1) var resp *pb.ChaincodeMessage diff --git a/core/chaincode/handler_test.go b/core/chaincode/handler_test.go index 62e5fc56575..7e929e81d38 100644 --- a/core/chaincode/handler_test.go +++ b/core/chaincode/handler_test.go @@ -490,6 +490,43 @@ var _ = Describe("Handler", func() { })) }) }) + + // Regression test for https://github.com/hyperledger/fabric/issues/5048 + // When a transaction times out during a range query, the timeout path closes + // the LevelDB iterator while the handler goroutine is still using it, causing + // a nil pointer dereference panic. The recover() in HandleTransaction prevents + // this from crashing the peer. + Context("when the delegate panics", func() { + It("recovers and sends an error response", func() { + panickingDelegate := func(msg *pb.ChaincodeMessage, txContext *chaincode.TransactionContext) (*pb.ChaincodeMessage, error) { + panic("simulated nil pointer dereference from closed iterator") + } + + Expect(func() { + handler.HandleTransaction(incomingMessage, panickingDelegate) + }).NotTo(Panic()) + + Eventually(fakeChatStream.SendCallCount).Should(Equal(1)) + msg := fakeChatStream.SendArgsForCall(0) + Expect(msg.Type).To(Equal(pb.ChaincodeMessage_ERROR)) + Expect(msg.Txid).To(Equal("tx-id")) + Expect(msg.ChannelId).To(Equal("channel-id")) + Expect(string(msg.Payload)).To(ContainSubstring("panic during execution")) + }) + + It("deregisters the transaction ID", func() { + panickingDelegate := func(msg *pb.ChaincodeMessage, txContext *chaincode.TransactionContext) (*pb.ChaincodeMessage, error) { + panic("simulated panic") + } + + handler.HandleTransaction(incomingMessage, panickingDelegate) + + Expect(fakeTransactionRegistry.RemoveCallCount()).To(Equal(1)) + channelID, transactionID := fakeTransactionRegistry.RemoveArgsForCall(0) + Expect(channelID).To(Equal("channel-id")) + Expect(transactionID).To(Equal("tx-id")) + }) + }) }) Describe("HandlePutState", func() {