signal: fix data race in logger during shutdown

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

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 @@
 // 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
+}

signal/signal.go

@@ -31,7 +31,7 @@
 		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 @@
 		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 @@
 
 	// 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 @@
 	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 @@
 	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