From a623fe1c0de706f07df27c2f7604df7023976b78 Mon Sep 17 00:00:00 2001 From: Josh Rickmar Date: Mon, 13 Nov 2023 16:41:52 +0000 Subject: [PATCH] peer: provide better debug for queued nil messages If the doneChan is nil, the nil message can be safely ignored. However, this is still a bug in the caller, so a warning with stacktrace is logged. If the doneChan is non-nil, panicing is the only reasonable option to prevent the program continuing on in an unknown state. The peer code would already panic previously in wire.WriteMessageN for many queued message types, and introducing a new panic call here should not introduce any new crashes that would not have occurred before. --- peer/peer.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/peer/peer.go b/peer/peer.go index e88f6f428e..30c0381ff7 100644 --- a/peer/peer.go +++ b/peer/peer.go @@ -1688,6 +1688,23 @@ cleanup: // // This function is safe for concurrent access. func (p *Peer) QueueMessage(msg wire.Message, doneChan chan<- struct{}) { + // Provide debug information when called with a nil message. This + // provides a more useful stack trace to callers than hitting the + // panic in a long-lived peer goroutine that contains no information + // about what caller queued the nil message. + if msg == nil { + // The semantics of whether a non-nil doneChan should be sent + // to or not, and the unknown program state this might lead + // to, doesn't leave a reasonable option to recover from this. + if doneChan != nil { + panic("peer: queued nil message") + } + + log.Warnf("Attempt to send nil message type %T\nStack: %s", + msg, debug.Stack()) + return + } + // Avoid risk of deadlock if goroutine already exited. The goroutine // we will be sending to hangs around until it knows for a fact that // it is marked as disconnected and *then* it drains the channels.