From 29b9c8bd136b6829674de7c364d36d93cef35255 Mon Sep 17 00:00:00 2001 From: Eduardo Bravo Date: Sat, 30 May 2026 20:58:55 -0500 Subject: [PATCH] signal: fix data race in logger during shutdown Add sync.RWMutex to protect concurrent access to the package-level logger variable. The mainInterruptHandler goroutine reads the logger while UseLogger() writes it during startup, causing a data race. Changes: - Add logMtx (sync.RWMutex) to protect log variable - Wrap UseLogger() writes with Lock/Unlock - Add getLogger() helper that uses RLock/RUnlock for thread-safe reads - Replace all direct log access in signal.go with getLogger() calls Fixes #6137 --- docs/release-notes/release-notes-0.21.0.md | 6 +++++ signal/log.go | 28 +++++++++++++++++++++- signal/signal.go | 20 ++++++++-------- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/docs/release-notes/release-notes-0.21.0.md b/docs/release-notes/release-notes-0.21.0.md index 2daafe61e3e..f64b7672daa 100644 --- a/docs/release-notes/release-notes-0.21.0.md +++ b/docs/release-notes/release-notes-0.21.0.md @@ -39,6 +39,12 @@ `maxchansize`. The `maxchansize` config option is intended only for limiting incoming channel requests from peers, not outgoing ones. +* [Fixed a data race](https://github.com/lightningnetwork/lnd/pull/10866) + in the signal package logger that occurred during shutdown when lnd was + built with the `-race` flag. The race happened because the + `mainInterruptHandler` goroutine could read the logger while the main + goroutine was replacing it during startup. + - Chain notifier RPCs now [return the gRPC `Unavailable` status](https://github.com/lightningnetwork/lnd/pull/10352) while the sub-server is still starting. This allows clients to reliably detect the diff --git a/signal/log.go b/signal/log.go index 4fcbd6d7aac..49cbf2386e3 100644 --- a/signal/log.go +++ b/signal/log.go @@ -1,12 +1,23 @@ package signal -import "github.com/btcsuite/btclog/v2" +import ( + "sync" + + "github.com/btcsuite/btclog/v2" +) // log is a logger that is initialized with no output filters. This // means the package will not perform any logging by default until the caller // requests it. var log btclog.Logger +// logMtx protects the log variable from concurrent access. This mutex is +// necessary because the mainInterruptHandler goroutine reads the logger while +// UseLogger may be called from the main goroutine during startup to replace +// the logger with a properly configured one. Without this protection, a data +// race would occur as reported in issue #6137. +var logMtx sync.RWMutex + // The default amount of logging is none. func init() { DisableLog() @@ -22,5 +33,20 @@ func DisableLog() { // This should be used in preference to SetLogWriter if the caller is also // using btclog. func UseLogger(logger btclog.Logger) { + logMtx.Lock() + defer logMtx.Unlock() log = logger } + +// getLogger returns the current logger in a thread-safe manner. This function +// must be used instead of directly accessing the log variable to prevent data +// races between the mainInterruptHandler goroutine and the main goroutine +// during logger initialization. +func getLogger() btclog.Logger { + logMtx.RLock() + defer logMtx.RUnlock() + if log == nil { + return btclog.Disabled + } + return log +} diff --git a/signal/signal.go b/signal/signal.go index 8b4a01485ce..d61d58f11a8 100644 --- a/signal/signal.go +++ b/signal/signal.go @@ -31,7 +31,7 @@ func systemdNotifyReady() error { err := fmt.Errorf("failed to notify systemd %v (if you aren't "+ "running systemd clear the environment variable "+ "NOTIFY_SOCKET)", err) - log.Error(err) + getLogger().Error(err) // The SdNotify doc says it's common to ignore the // error. We don't want to ignore it because if someone @@ -40,9 +40,9 @@ func systemdNotifyReady() error { return err } if notified { - log.Info("Systemd was notified about our readiness") + getLogger().Info("Systemd was notified about our readiness") } else { - log.Info("We're not running within systemd or the service " + + getLogger().Info("We're not running within systemd or the service " + "type is not 'notify'") } return nil @@ -56,10 +56,10 @@ func systemdNotifyStop() { // Just log - we're stopping anyway. if err != nil { - log.Errorf("Failed to notify systemd: %v", err) + getLogger().Errorf("Failed to notify systemd: %v", err) } if notified { - log.Infof("Systemd was notified about stopping") + getLogger().Infof("Systemd was notified about stopping") } } @@ -159,11 +159,11 @@ func (c *Interceptor) mainInterruptHandler() { shutdown := func() { // Ignore more than one shutdown signal. if isShutdown { - log.Infof("Already shutting down...") + getLogger().Infof("Already shutting down...") return } isShutdown = true - log.Infof("Shutting down...") + getLogger().Infof("Shutting down...") c.Notifier.notifyStop() // Signal the main interrupt handler to exit, and stop accept @@ -174,15 +174,15 @@ func (c *Interceptor) mainInterruptHandler() { for { select { case signal := <-c.interruptChannel: - log.Infof("Received %v", signal) + getLogger().Infof("Received %v", signal) shutdown() case <-c.shutdownRequestChannel: - log.Infof("Received shutdown request.") + getLogger().Infof("Received shutdown request.") shutdown() case <-c.quit: - log.Infof("Gracefully shutting down.") + getLogger().Infof("Gracefully shutting down.") close(c.shutdownChannel) signal.Stop(c.interruptChannel) return