multi: standardize panic recovery at async boundaries

actor/actor.go

@@ -2,6 +2,8 @@ package actor
 
 import (
 	"context"
+	"fmt"
+	"log/slog"
 	"sync"
 
 	"github.com/lightningnetwork/lnd/fn/v2"
@@ -144,14 +146,7 @@ func (a *Actor[M, R]) Start() {
 func (a *Actor[M, R]) process() {
 	// Use the new iterator pattern for receiving messages.
 	for env := range a.mailbox.Receive(a.ctx) {
-		result := a.behavior.Receive(a.ctx, env.message)
-
-		// If a promise was provided (i.e., it was an "ask"
-		// operation), complete the promise with the result from
-		// the behavior.
-		if env.promise != nil {
-			env.promise.Complete(result)
-		}
+		a.deliver(env)
 	}
 
 	// Context was cancelled or mailbox closed, drain remaining messages.
@@ -172,6 +167,39 @@ func (a *Actor[M, R]) process() {
 	}
 }
 
+// deliver hands a single message to the actor's behavior, completing the
+// envelope's promise with whatever the behavior returned.
+//
+// A recovered behavior panic completes any pending promise with ErrActorPanic.
+// The actor can then continue with the remaining mailbox entries.
+func (a *Actor[M, R]) deliver(env envelope[M, R]) {
+	defer fn.RecoverPanic(func(pnc fn.Panic) {
+		fn.LogRecoveredPanic(
+			a.ctx, log, pnc,
+			slog.String("actor_id", a.id),
+			// Log the concrete Go type instead of calling a method on the
+			// message while already recovering from a panic.
+			slog.String(
+				"message_type", fmt.Sprintf("%T", env.message),
+			),
+		)
+
+		if env.promise != nil {
+			env.promise.Complete(fn.Err[R](
+				fmt.Errorf("%w: %v", ErrActorPanic, pnc.Value),
+			))
+		}
+	})
+
+	result := a.behavior.Receive(a.ctx, env.message)
+
+	// If a promise was provided (i.e., it was an "ask" operation), complete
+	// the promise with the result from the behavior.
+	if env.promise != nil {
+		env.promise.Complete(result)
+	}
+}
+
 // Stop signals the actor to terminate its processing loop and shut down.
 // This is achieved by cancelling the actor's internal context. The actor's
 // goroutine will exit once it detects the context cancellation.

actor/actor_test.go

@@ -120,6 +120,24 @@ func (b *blockingBehavior) Receive(actorCtx context.Context,
 	return fn.Err[string](actorCtx.Err())
 }
 
+// panicBehavior is an actor behavior that panics on the first message it
+// receives, then echoes every message after that.
+type panicBehavior struct {
+	panicked atomic.Bool
+}
+
+// Receive panics the first time it's called, and echoes the message data on
+// every call after that.
+func (b *panicBehavior) Receive(_ context.Context,
+	msg *testMsg) fn.Result[string] {
+
+	if b.panicked.CompareAndSwap(false, true) {
+		panic("behavior panic")
+	}
+
+	return fn.Ok(fmt.Sprintf("echo: %s", msg.data))
+}
+
 // deadLetterTestMsg is a distinct message type used for testing DLO
 // interactions.
 type deadLetterTestMsg struct {
@@ -392,6 +410,41 @@ func TestActorAskErrorBehavior(t *testing.T) {
 	h.assertNoDLOMessages()
 }
 
+// TestActorBehaviorPanic verifies that a behavior panic completes the pending
+// request with ErrActorPanic and that later messages are processed.
+func TestActorBehaviorPanic(t *testing.T) {
+	t.Parallel()
+
+	h := newActorTestHarness(t)
+	beh := &panicBehavior{}
+	actor := h.newActor("test-actor-panic", beh, 1)
+
+	// The first message makes the behavior panic. The pending request should
+	// complete with the recovery error.
+	future := actor.Ref().Ask(context.Background(), newTestMsg("panic-msg"))
+
+	awaitCtx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
+	result := future.Await(awaitCtx)
+	cancel()
+	require.True(t, result.IsErr(), "ask should have returned an error")
+	require.ErrorIs(t, result.Err(), ErrActorPanic, "ask error mismatch")
+
+	// The actor should still be alive and serving, so the next message is
+	// echoed as usual.
+	future = actor.Ref().Ask(context.Background(), newTestMsg("live-msg"))
+
+	awaitCtx, cancel = context.WithTimeout(t.Context(), 5*time.Second)
+	result = future.Await(awaitCtx)
+	cancel()
+	require.False(t, result.IsErr(), "ask returned an error: %v",
+		result.Err())
+	require.Equal(t, fn.Ok("echo: live-msg"), result)
+
+	// A panicking message is dropped rather than dead lettered, as the actor
+	// itself is still healthy.
+	h.assertNoDLOMessages()
+}
+
 // TestFunctionBehaviorFromSimple verifies that FunctionBehaviorFromSimple
 // correctly adapts a simple (msg) -> (result, error) function into an
 // ActorBehavior, handling both success and error cases.

actor/interface.go

@@ -16,6 +16,11 @@ var ErrActorTerminated = fmt.Errorf("actor terminated")
 // backpressure mechanism (e.g., RED-style load shedding).
 var ErrMessageDropped = errors.New("message dropped by backpressure")
 
+// ErrActorPanic indicates that the actor's behavior panicked while processing a
+// message. The message is dropped, and the actor remains available to serve the
+// messages that follow it.
+var ErrActorPanic = errors.New("actor panicked while processing message")
+
 // ErrEmptyActorID is returned when an actor is created with an empty ID.
 var ErrEmptyActorID = fmt.Errorf("actor ID must not be empty")
 

discovery/gossiper.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"runtime/debug"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -1739,22 +1738,15 @@ func (d *AuthenticatedGossiper) finalizeGossipProcessing(logCtx context.Context,
 		peerPub = "unknown"
 	}
 
-	log.ErrorS(logCtx, "Panic during gossip message processing",
-		fmt.Errorf("%v", r),
+	fn.LogRecoveredPanic(
+		logCtx, log, fn.Panic{
+			Value: r,
+			Stack: fn.PanicStack(),
+		},
 		slog.String("context", ctxStr),
 		slog.String("msg_type", msgType),
 		slog.String("peer", peerPub),
 	)
-	// Truncate the stack trace to avoid filling up disk space if an
-	// attacker repeatedly triggers panics.
-	const maxStackSize = 8192
-	stack := debug.Stack()
-	if len(stack) > maxStackSize {
-		stack = stack[:maxStackSize]
-	}
-	log.DebugS(logCtx, "Panic stack trace",
-		slog.String("stack", string(stack)),
-	)
 
 	// Signal any dependents waiting on this message so they don't block
 	// forever.

docs/release-notes/release-notes-0.20.4.md

@@ -71,6 +71,13 @@
 
 ## Code Health
 
+* Critical long-running subsystems [now recover from unexpected handler
+  panics](https://github.com/lightningnetwork/lnd/pull/11097) instead of
+  terminating `lnd`. Recovered failures log a bounded stack trace and follow
+  subsystem-specific containment so affected peers, links, state machines and
+  requests fail safely while unrelated work can continue. This is a containment
+  backstop; the underlying panic remains visible in the logs for diagnosis.
+
 ## Tooling and Documentation
 
 # Contributors (Alphabetical Order)

docs/release-notes/release-notes-0.21.3.md

@@ -96,6 +96,13 @@
 
 ## Code Health
 
+* Critical long-running subsystems [now recover from unexpected handler
+  panics](https://github.com/lightningnetwork/lnd/pull/11097) instead of
+  terminating `lnd`. Recovered failures log a bounded stack trace and follow
+  subsystem-specific containment so affected peers, links, state machines and
+  requests fail safely while unrelated work can continue. This is a containment
+  backstop; the underlying panic remains visible in the logs for diagnosis.
+
 ## Tooling and Documentation
 
 # Contributors (Alphabetical Order)

fn/stack.go

@@ -0,0 +1,134 @@
+package fn
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"log/slog"
+	"runtime/debug"
+)
+
+const (
+	// maxPanicStackSize is the largest recovered-panic stack trace returned
+	// by these helpers. Bounding it keeps repeated reports from consuming
+	// excessive log space.
+	maxPanicStackSize = 8192
+
+	// panicStackTruncatedMsg is appended in place of whatever we cut, so
+	// that whoever reads the log can tell the trace is incomplete instead
+	// of assuming it ended where it stops.
+	panicStackTruncatedMsg = "\n... stack trace truncated ..."
+)
+
+// errRecoveredPanic is the sentinel error attached to every structured log
+// record emitted for a recovered panic.
+var errRecoveredPanic = errors.New("recovered panic")
+
+// Panic carries the details of a panic we stopped: the value handed to panic(),
+// and a stack trace bounded to a size that's safe to log. The trace still
+// covers the frames the panic came from, not just the point we caught it at.
+type Panic struct {
+	// Value is whatever was passed to panic().
+	Value any
+
+	// Stack is the stack trace of the panicking goroutine, truncated to a
+	// bounded size.
+	Stack []byte
+}
+
+// PanicLogger is the minimal structured-logging surface needed to report a
+// recovered panic. Keeping the interface here avoids coupling fn to a concrete
+// logging package.
+type PanicLogger interface {
+	ErrorS(context.Context, string, error, ...any)
+}
+
+// LogRecoveredPanic records a recovered panic in a consistent structured
+// format. The logger's subsystem and the captured stack identify where the
+// panic was recovered. Callers can attach domain-specific context as additional
+// structured attributes.
+//
+// Reporting is best effort. A panic while formatting or writing the record is
+// recovered so the reporting path can return normally.
+func LogRecoveredPanic(ctx context.Context, logger PanicLogger,
+	p Panic, extraAttrs ...any) {
+
+	if logger == nil {
+		return
+	}
+
+	// A panic in the logging path must not interrupt the caller's panic
+	// containment and cleanup.
+	defer func() {
+		_ = recover()
+	}()
+
+	attrs := []any{
+		slog.String("panic_type", fmt.Sprintf("%T", p.Value)),
+		slog.Any("panic_value", p.Value),
+		slog.String("stack", string(p.Stack)),
+	}
+	attrs = append(attrs, extraAttrs...)
+
+	logger.ErrorS(
+		ctx, "Recovered panic", errRecoveredPanic, attrs...,
+	)
+}
+
+// RecoverPanic stops a panic from unwinding the calling goroutine any further
+// and hands the details to onPanic. If the goroutine isn't panicking, onPanic
+// is never called and no stack is captured.
+//
+// This MUST be deferred directly:
+//
+//	defer fn.RecoverPanic(func(p fn.Panic) {
+//	        log.Errorf("recovered: %v\n%s", p.Value, p.Stack)
+//	})
+//
+// Wrapping it in a closure instead, as in
+// `defer func() { fn.RecoverPanic(..) }()`, silently does nothing: the language
+// only lets recover() stop a panic when it's called by the deferred function
+// itself, so one frame further down it returns nil and the panic carries on
+// unwinding.
+func RecoverPanic(onPanic func(Panic)) {
+	r := recover()
+	if r == nil {
+		return
+	}
+
+	onPanic(Panic{
+		Value: r,
+		Stack: TruncatePanicStack(debug.Stack()),
+	})
+}
+
+// PanicStack returns the stack trace of the calling goroutine, bounded to the
+// configured size.
+func PanicStack() []byte {
+	return TruncatePanicStack(debug.Stack())
+}
+
+// TruncatePanicStack caps a panic stack trace at a bounded size. Where it can,
+// it cuts on a line boundary so the last frame in the log stays readable rather
+// than ending mid-token, and it marks the cut so the trace doesn't look
+// complete.
+func TruncatePanicStack(stack []byte) []byte {
+	if len(stack) <= maxPanicStackSize {
+		return stack
+	}
+
+	suffix := []byte(panicStackTruncatedMsg)
+	maxStackLen := maxPanicStackSize - len(suffix)
+	searchStack := stack[:maxStackLen+1]
+	newLineIndex := bytes.LastIndexByte(searchStack, '\n')
+	if newLineIndex > 0 {
+		maxStackLen = newLineIndex
+	}
+
+	truncatedStack := make([]byte, 0, maxStackLen+len(suffix))
+	truncatedStack = append(truncatedStack, stack[:maxStackLen]...)
+	truncatedStack = append(truncatedStack, suffix...)
+
+	return truncatedStack
+}