From 3df2b50a90eb73b5e67ea85e6e58e4c05fe4a0e6 Mon Sep 17 00:00:00 2001
From: Josh Rickmar <jrick@companyzero.com>
Date: Mon, 13 Nov 2023 16:43:42 +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 | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/peer/peer.go b/peer/peer.go
index e88f6f428e..e03302cb6f 100644
--- a/peer/peer.go
+++ b/peer/peer.go
@@ -12,6 +12,7 @@ import (
 	"io"
 	"math/rand"
 	"net"
+	"runtime/debug"
 	"strconv"
 	"sync"
 	"sync/atomic"
@@ -1688,6 +1689,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.