diff --git a/config.go b/config.go index fc6fd7f93..b5b860ffe 100644 --- a/config.go +++ b/config.go @@ -579,7 +579,8 @@ func defaultConfig() *Config { // loadAndValidateConfig loads the terminal's main configuration and validates // its content. func loadAndValidateConfig(ctx context.Context, - interceptor signal.Interceptor) (*Config, error) { + interceptor signal.Interceptor, + litdShutdown *shutdownSource) (*Config, error) { // Start with the default configuration. preCfg := defaultConfig() @@ -614,7 +615,7 @@ func loadAndValidateConfig(ctx context.Context, preCfg.Lnd.SubLogMgr = build.NewSubLoggerManager( build.NewDefaultLogHandlers(logCfg, preCfg.Lnd.LogRotator)..., ) - SetupLoggers(preCfg.Lnd.SubLogMgr, interceptor) + SetupLoggers(preCfg.Lnd.SubLogMgr, interceptor, litdShutdown) // Load the main configuration file and parse any command line options. // This function will also set up logging properly. diff --git a/docs/release-notes/release-notes-0.17.1.md b/docs/release-notes/release-notes-0.17.1.md index 6627288ed..a1f83c12a 100644 --- a/docs/release-notes/release-notes-0.17.1.md +++ b/docs/release-notes/release-notes-0.17.1.md @@ -25,8 +25,23 @@ ### Functional Changes/Additions +* [Keep litd running after lnd + stops](https://github.com/lightninglabs/lightning-terminal/pull/1329): + In integrated mode, litd no longer shuts down when lnd stops. It now marks lnd + as errored, tears down the lnd dependent sub-servers and keeps running so its + status endpoint stays available to report what happened. Note that calling + lnd's `StopDaemon` (for example `lncli stop`) no longer stops litd; use litd's + own `StopDaemon` to stop everything. + ### Technical and Architectural Updates +* [Stop itest litd nodes via litd's own + `StopDaemon`](https://github.com/lightninglabs/lightning-terminal/pull/1356): + The itest harness now sets up a lit connection for every node, seed based or + not, so nodes are stopped through litd's own `StopDaemon`. For stateless init + nodes, which keep no macaroon files on disk, the harness bakes a lit super + macaroon from the admin macaroon to authenticate that call. + * [Report litd's own version for `litd -V`](https://github.com/lightninglabs/lightning-terminal/pull/1337): The `-V` flag now prints litd's version instead of the integrated lnd version. diff --git a/itest/litd_lnd_shutdown_test.go b/itest/litd_lnd_shutdown_test.go new file mode 100644 index 000000000..3c5ae7db0 --- /dev/null +++ b/itest/litd_lnd_shutdown_test.go @@ -0,0 +1,129 @@ +package itest + +import ( + "context" + "fmt" + + "github.com/lightninglabs/lightning-terminal/litrpc" + "github.com/lightninglabs/lightning-terminal/subservers" + "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lntest/wait" + "github.com/stretchr/testify/require" +) + +// testLitdSurvivesLndShutdown asserts that when lnd stops in integrated mode, +// litd keeps running and its status endpoint stays reachable, reporting lnd as +// stopped rather than cascading to a full litd shutdown. +func testLitdSurvivesLndShutdown(ctx context.Context, net *NetworkHarness, + t *harnessTest) { + + // Use a dedicated, throwaway node so that stopping its lnd cannot + // affect any other test. + node, err := net.NewNode( + t.t, "lnd-stop", nil, false, true, "--autopilot.disable", + ) + require.NoError(t.t, err) + + // The harness ShutdownNode stops litd (via its StopDaemon) and waits + // for the process to exit, so no manual litd shutdown is needed here. + defer func() { + _ = net.ShutdownNode(node) + }() + + ctxt, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + rawConn, err := connectLitRPC( + ctxt, node.Cfg.LitAddr(), node.Cfg.LitTLSCertPath, + node.Cfg.LitMacPath, + ) + require.NoError(t.t, err) + defer func() { _ = rawConn.Close() }() + + statusClient := litrpc.NewStatusClient(rawConn) + + // lnd should be running before we stop it. + resp, err := statusClient.SubServerStatus( + ctxt, &litrpc.SubServerStatusReq{}, + ) + require.NoError(t.t, err) + + lndStatus, ok := resp.SubServers[subservers.LND] + require.True(t.t, ok, "expected lnd sub-server status") + require.True(t.t, lndStatus.Running, "lnd should be running initially") + + // Stop lnd (but not litd) via the lnrpc StopDaemon. The call may return + // an error as the connection is torn down, which is expected. + _, _ = node.LightningClient.StopDaemon(ctxt, &lnrpc.StopRequest{}) + + // litd's status endpoint must remain reachable and must report lnd as + // no longer running, with an error explaining why. We poll because lnd + // shutdown is asynchronous. + err = wait.NoError(func() error { + ctxs, cancels := context.WithTimeout( + context.Background(), defaultTimeout, + ) + defer cancels() + + resp, err := statusClient.SubServerStatus( + ctxs, &litrpc.SubServerStatusReq{}, + ) + if err != nil { + return fmt.Errorf("litd status endpoint unreachable "+ + "after lnd stopped: %w", err) + } + + lndStatus, ok := resp.SubServers[subservers.LND] + if !ok { + return fmt.Errorf("no lnd sub-server status returned") + } + + if lndStatus.Running { + return fmt.Errorf("lnd still reported as running") + } + + if lndStatus.Error == "" { + return fmt.Errorf("lnd error message not yet set") + } + + return nil + }, defaultTimeout) + require.NoError(t.t, err) + + // litd's process itself must keep running after lnd has stopped: its + // status endpoint stays reachable. The LIT sub-server is reported as no + // longer running, since litd cannot function without lnd, but the + // endpoint continuing to answer at all is what proves litd did not + // cascade down with lnd. Check this holds over time. + err = wait.InvariantNoError(func() error { + ctxs, cancels := context.WithTimeout( + context.Background(), defaultTimeout, + ) + defer cancels() + + resp, err := statusClient.SubServerStatus( + ctxs, &litrpc.SubServerStatusReq{}, + ) + if err != nil { + return fmt.Errorf("litd status endpoint must stay "+ + "reachable after lnd stopped: %w", err) + } + + litStatus, ok := resp.SubServers[subservers.LIT] + if !ok { + return fmt.Errorf("no lit sub-server status returned") + } + + if litStatus.Running { + return fmt.Errorf("lit sub-server should be reported " + + "as stopped once lnd has stopped") + } + + if litStatus.Error == "" { + return fmt.Errorf("lit sub-server error message not set") + } + + return nil + }, defaultTimeout) + require.NoError(t.t, err) +} diff --git a/itest/litd_node.go b/itest/litd_node.go index 753b14ed1..d0eb691ad 100644 --- a/itest/litd_node.go +++ b/itest/litd_node.go @@ -45,7 +45,10 @@ import ( "github.com/lightningnetwork/lnd/lntest/wait" "github.com/lightningnetwork/lnd/macaroons" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" "gopkg.in/macaroon.v2" ) @@ -384,6 +387,13 @@ type HarnessNode struct { // litConn is the underlying connection to Lit's grpc endpoint. litConn *grpc.ClientConn + // litMacPath, when set, is the macaroon file used to authenticate + // litConn. It is set for stateless init nodes, which have no lit + // macaroon on disk: we bake a super macaroon from the in-memory admin + // macaroon and point this at it so the node can still be stopped via + // litd's own StopDaemon. + litMacPath string + // RouterClient, WalletKitClient, WatchtowerClient cannot be embedded, // because a name collision would occur with LightningClient. RouterClient routerrpc.RouterClient @@ -776,31 +786,9 @@ func (hn *HarnessNode) Start(litdBinary string, litdError chan<- error, return nil } - err = hn.initLightningClient(conn) - if err != nil { - return fmt.Errorf("could not init Lightning Client: %w", err) - } - - // Also connect to Lit's RPC port for any Litd specific calls. - litConn, err := connectLitRPC( - context.Background(), hn.Cfg.LitAddr(), hn.Cfg.LitTLSCertPath, - hn.Cfg.LitMacPath, - ) - if err != nil { - return fmt.Errorf("could not connect to Lit RPC: %w", err) - } - hn.litConn = litConn - - ctxt, cancel := context.WithTimeout( - context.Background(), lntest.DefaultTimeout, - ) - defer cancel() - return wait.NoError(func() error { - litConn := litrpc.NewProxyClient(hn.litConn) - - _, err = litConn.GetInfo(ctxt, &litrpc.GetInfoRequest{}) - return err - }, lntest.DefaultTimeout) + // initLightningClient also connects to Lit's RPC port and sets + // hn.litConn so the node can be stopped via litd's own StopDaemon. + return hn.initLightningClient(conn) } // WaitForLNDWalletReady waits until the wallet state flips from @@ -1087,6 +1075,60 @@ func (hn *HarnessNode) initClientWhenReady(timeout time.Duration) error { return hn.initLightningClient(conn) } +// bakeLitSuperMacaroon bakes a lit super macaroon from the given admin macaroon +// (returned in-memory during stateless init, where no lit macaroon is written +// to disk), writes it next to the node's lit config, and records its path so +// litConn can authenticate against it. This lets the harness stop a stateless +// node via litd's own StopDaemon. +func (hn *HarnessNode) bakeLitSuperMacaroon(adminMac []byte) error { + ctxt, cancel := context.WithTimeout( + context.Background(), lntest.DefaultTimeout, + ) + defer cancel() + + rawConn, err := connectLitRPC( + ctxt, hn.Cfg.LitAddr(), hn.Cfg.LitTLSCertPath, "", + ) + if err != nil { + return err + } + defer rawConn.Close() + + // Attach the admin macaroon to the request context so litd authorizes + // the bake call. + macCtx := metadata.NewOutgoingContext(ctxt, metadata.MD{ + "macaroon": []string{hex.EncodeToString(adminMac)}, + }) + + litConn := litrpc.NewProxyClient(rawConn) + resp, err := litConn.BakeSuperMacaroon( + macCtx, &litrpc.BakeSuperMacaroonRequest{ + RootKeyIdSuffix: 0, + ReadOnly: false, + }, + ) + if err != nil { + return err + } + + // BakeSuperMacaroon hex encodes the returned macaroon. + superMacBytes, err := hex.DecodeString(resp.Macaroon) + if err != nil { + return err + } + + macPath := filepath.Join( + filepath.Dir(hn.Cfg.LitMacPath), "lit-super.macaroon", + ) + if err := os.WriteFile(macPath, superMacBytes, 0644); err != nil { + return err + } + + hn.litMacPath = macPath + + return nil +} + // Init initializes a harness node by passing the init request via rpc. After // the request is submitted, this method will block until a // macaroon-authenticated RPC connection can be established to the harness node. @@ -1126,6 +1168,17 @@ func (hn *HarnessNode) Init(ctx context.Context, return nil, err } + // In stateless init mode no lit macaroon is written to disk, so bake a + // super macaroon from the admin macaroon we just received. The harness + // uses it to stop the node via litd's own StopDaemon. + if initReq.StatelessInit { + err := hn.bakeLitSuperMacaroon(response.AdminMacaroon) + if err != nil { + return nil, fmt.Errorf("could not bake lit super "+ + "macaroon: %w", err) + } + } + return response, hn.initLightningClient(conn) } @@ -1169,6 +1222,17 @@ func (hn *HarnessNode) InitChangePassword(ctx context.Context, return nil, err } + // In stateless init mode no lit macaroon is written to disk, so bake a + // super macaroon from the admin macaroon we just received. The harness + // uses it to stop the node via litd's own StopDaemon. + if chngPwReq.StatelessInit { + err := hn.bakeLitSuperMacaroon(response.AdminMacaroon) + if err != nil { + return nil, fmt.Errorf("could not bake lit super "+ + "macaroon: %w", err) + } + } + return response, hn.initLightningClient(conn) } @@ -1252,6 +1316,41 @@ func (hn *HarnessNode) initLightningClient(conn *grpc.ClientConn) error { return err } + // Also connect to Lit's RPC port for any litd specific calls. We do + // this here, rather than only on the no-seed path in start, so that + // seed based nodes (which reach this method via Init or Unlock) get a + // litConn too. Stop relies on it to shut the node down via litd's own + // StopDaemon, which is the only way to bring litd down now that its + // lifecycle is decoupled from lnd's. For stateless nodes litMacPath + // points at a baked super macaroon; otherwise the lit macaroon on disk + // is used. + macPath := hn.Cfg.LitMacPath + if hn.litMacPath != "" { + macPath = hn.litMacPath + } + litConn, err := connectLitRPC( + context.Background(), hn.Cfg.LitAddr(), hn.Cfg.LitTLSCertPath, + macPath, + ) + if err != nil { + return fmt.Errorf("could not connect to Lit RPC: %w", err) + } + hn.litConn = litConn + + ctxt, cancel := context.WithTimeout( + context.Background(), lntest.DefaultTimeout, + ) + defer cancel() + err = wait.NoError(func() error { + litClient := litrpc.NewProxyClient(hn.litConn) + + _, err := litClient.GetInfo(ctxt, &litrpc.GetInfoRequest{}) + return err + }, lntest.DefaultTimeout) + if err != nil { + return err + } + // Launch the watcher that will hook into graph related topology change // from the PoV of this node. hn.wg.Add(1) @@ -1423,9 +1522,31 @@ func (hn *HarnessNode) Stop() error { return nil } - // If start() failed before creating a client, we will just wait for the - // child process to die. - if !hn.Cfg.RemoteMode && hn.LightningClient != nil { + // litd owns its own lifecycle, so we ask it to stop via its own + // StopDaemon RPC. In integrated mode this also brings down the embedded + // lnd. + switch { + case hn.litConn != nil: + ctx, cancel := context.WithTimeout( + context.Background(), lntest.DefaultTimeout, + ) + litConn := litrpc.NewProxyClient(hn.litConn) + + // litd normally answers before it shuts down, but the + // connection can also be torn down first as litd begins + // shutting down. That surfaces as an Unavailable status and + // is expected. Any other error is not, so surface it. We + // confirm the process actually exits via processExit below. + _, err := litConn.StopDaemon(ctx, &litrpc.StopDaemonRequest{}) + cancel() + if err != nil && status.Code(err) != codes.Unavailable { + return err + } + + // If start() failed before the lit connection was created, fall back to + // stopping the integrated lnd directly and just wait for the child + // process to die. + case !hn.Cfg.RemoteMode && hn.LightningClient != nil: // Don't watch for error because sometimes the RPC connection // gets closed before a response is returned. req := lnrpc.StopRequest{} @@ -1450,19 +1571,6 @@ func (hn *HarnessNode) Stop() error { if err != nil { return err } - } else if hn.Cfg.RemoteMode { - // If lit is running in remote mode, then calling LNDs - // StopDaemon method will not shut down Lit, and so we need to - // explicitly request lit to shut down. - ctx, cancel := context.WithTimeout( - context.Background(), lntest.DefaultTimeout, - ) - litConn := litrpc.NewProxyClient(hn.litConn) - _, err := litConn.StopDaemon(ctx, &litrpc.StopDaemonRequest{}) - cancel() - if err != nil { - return err - } } // Wait for litd process and other goroutines to exit. diff --git a/itest/litd_test_list_on_test.go b/itest/litd_test_list_on_test.go index eaf0f77fe..2460cf18b 100644 --- a/itest/litd_test_list_on_test.go +++ b/itest/litd_test_list_on_test.go @@ -31,4 +31,8 @@ var allTestCases = []*testCase{ name: "kvdb to sql migration", test: testKvdbSQLMigration, }, + { + name: "litd survives lnd shutdown", + test: testLitdSurvivesLndShutdown, + }, } diff --git a/log.go b/log.go index dedd74dc2..4f944c209 100644 --- a/log.go +++ b/log.go @@ -61,9 +61,13 @@ func UseLogger(logger btclog.Logger) { log = logger } -// SetupLoggers initializes all package-global logger variables. -func SetupLoggers(root *build.SubLoggerManager, intercept signal.Interceptor) { - genLogger := genSubLogger(root, intercept) +// SetupLoggers initializes all package-global logger variables. The passed +// litdShutdown is litd's own shutdown source, used to bring litd down on a +// critical log independently of lnd's signal.Interceptor. +func SetupLoggers(root *build.SubLoggerManager, intercept signal.Interceptor, + litdShutdown *shutdownSource) { + + genLogger := genSubLogger(root, intercept, litdShutdown) log = build.NewSubLogger(Subsystem, genLogger) interceptor = intercept @@ -103,17 +107,26 @@ func SetupLoggers(root *build.SubLoggerManager, intercept signal.Interceptor) { tap.SetupLoggers(root, intercept) // Setup the gRPC loggers too. - grpclog.SetLoggerV2(NewGrpcLogLogger(root, intercept, GrpcLogSubsystem)) + grpclog.SetLoggerV2(NewGrpcLogLogger( + root, intercept, litdShutdown, GrpcLogSubsystem, + )) } // genSubLogger creates a logger for a subsystem. We provide an instance of // a signal.Interceptor to be able to shutdown in the case of a critical error. func genSubLogger(root *build.SubLoggerManager, - interceptor signal.Interceptor) func(string) btclog.Logger { + interceptor signal.Interceptor, + litdShutdown *shutdownSource) func(string) btclog.Logger { // Create a shutdown function which will request shutdown from our - // interceptor if it is listening. + // interceptor if it is listening. We also request shutdown from litd's + // own shutdown source so that a critical error brings litd down too, + // not just lnd (litd no longer exits simply because lnd stopped). shutdown := func() { + if litdShutdown != nil { + litdShutdown.RequestShutdown() + } + if !interceptor.Listening() { return } @@ -131,9 +144,12 @@ func genSubLogger(root *build.SubLoggerManager, // NewGrpcLogLogger creates a new grpclog compatible logger and attaches it as // a sub logger to the passed root logger. func NewGrpcLogLogger(root *build.SubLoggerManager, - intercept signal.Interceptor, subsystem string) *GrpcLogLogger { + intercept signal.Interceptor, litdShutdown *shutdownSource, + subsystem string) *GrpcLogLogger { - logger := build.NewSubLogger(subsystem, genSubLogger(root, intercept)) + logger := build.NewSubLogger( + subsystem, genSubLogger(root, intercept, litdShutdown), + ) lnd.SetSubLogger(root, subsystem, logger) return &GrpcLogLogger{ Logger: logger, diff --git a/rpc_proxy.go b/rpc_proxy.go index 950b527be..2f127c735 100644 --- a/rpc_proxy.go +++ b/rpc_proxy.go @@ -73,7 +73,8 @@ func (e *proxyErr) Unwrap() error { func newRpcProxy(cfg *Config, validator macaroons.MacaroonValidator, superMacValidator litmac.SuperMacaroonValidator, permsMgr *perms.Manager, subServerMgr *subservers.Manager, - statusMgr *litstatus.Manager, getLNDClient lndBasicClientFn) *rpcProxy { + statusMgr *litstatus.Manager, getLNDClient lndBasicClientFn, + litdShutdown *shutdownSource) *rpcProxy { // The gRPC web calls are protected by HTTP basic auth which is defined // by base64(username:password). Because we only have a password, we @@ -96,6 +97,7 @@ func newRpcProxy(cfg *Config, validator macaroons.MacaroonValidator, subServerMgr: subServerMgr, statusMgr: statusMgr, getBasicLNDClient: getLNDClient, + litdShutdown: litdShutdown, } p.grpcServer = grpc.NewServer( // From the grpxProxy doc: This codec is *crucial* to the @@ -173,6 +175,10 @@ type rpcProxy struct { statusMgr *litstatus.Manager getBasicLNDClient lndBasicClientFn + // litdShutdown is litd's own shutdown source. It is needed alongside + // the interceptor because litd's lifecycle is decoupled from lnd's. + litdShutdown *shutdownSource + bakeSuperMac bakeSuperMac macValidator macaroons.MacaroonValidator @@ -228,6 +234,11 @@ func (p *rpcProxy) StopDaemon(_ context.Context, log.Infof("StopDaemon rpc request received") + // Stop litd via its own shutdown source, and stop the embedded lnd via + // its interceptor. Both are needed because litd's lifecycle is + // decoupled from lnd's interceptor. + p.litdShutdown.RequestShutdown() + interceptor.RequestShutdown() return &litrpc.StopDaemonResponse{}, nil diff --git a/shutdown.go b/shutdown.go new file mode 100644 index 000000000..9dae82ecb --- /dev/null +++ b/shutdown.go @@ -0,0 +1,85 @@ +package terminal + +import ( + "os" + "os/signal" + "sync" + "syscall" +) + +// shutdownSource is litd's own shutdown trigger. It is intentionally separate +// from the signal.Interceptor that litd hands to lnd: lnd and litd are forced +// to share a single signal.Interceptor (lnd's signal package only allows one +// interceptor and lnd.Main takes it by value), so litd cannot tell an lnd +// shutdown apart from its own by watching that interceptor's channel. Owning +// its own shutdown source lets litd decide its own lifecycle and keep running +// (e.g. to serve its status endpoint) after lnd alone has stopped. +// +// litd is shut down when: +// - an OS interrupt or termination signal is received, +// - litd's StopDaemon RPC is called, or +// - a fatal litd-specific error occurs. +// +// It is NOT shut down when lnd alone stops. +type shutdownSource struct { + // quit is closed exactly once when litd should shut down. + quit chan struct{} + + // once guards closing quit so that RequestShutdown is idempotent. + once sync.Once + + // signals is the channel OS signals are delivered on. We keep a + // reference to it so it can be deregistered in Stop. + signals chan os.Signal +} + +// newShutdownSource constructs a shutdownSource and starts listening for OS +// interrupt and termination signals. +// +// Note that Go delivers a signal to every channel registered via +// signal.Notify, so listening here does not prevent lnd's own +// signal.Interceptor from also receiving the same signal. An OS signal +// therefore stops both lnd and litd. +func newShutdownSource() *shutdownSource { + s := &shutdownSource{ + quit: make(chan struct{}), + signals: make(chan os.Signal, 1), + } + + signal.Notify(s.signals, os.Interrupt, syscall.SIGTERM) + + go func() { + select { + case <-s.signals: + s.RequestShutdown() + + // If shutdown was requested by some other means, there is + // nothing left to wait for. + case <-s.quit: + } + }() + + return s +} + +// RequestShutdown closes the shutdown channel, signalling that litd should shut +// down. It is safe to call multiple times and from multiple goroutines. +func (s *shutdownSource) RequestShutdown() { + s.once.Do(func() { + close(s.quit) + }) +} + +// ShutdownChannel returns the channel that is closed when litd should shut +// down. The same channel is returned on every call, so callers may select on +// it repeatedly without spawning extra goroutines. +func (s *shutdownSource) ShutdownChannel() <-chan struct{} { + return s.quit +} + +// Stop deregisters the OS signal handler. It does not itself request a +// shutdown. It is mainly useful so that tests don't leave signal handlers +// registered on the process. +func (s *shutdownSource) Stop() { + signal.Stop(s.signals) +} diff --git a/shutdown_test.go b/shutdown_test.go new file mode 100644 index 000000000..fd9c6094c --- /dev/null +++ b/shutdown_test.go @@ -0,0 +1,67 @@ +package terminal + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// waitClosed asserts that the given channel is closed within a short timeout. +func waitClosed(t *testing.T, c <-chan struct{}) { + t.Helper() + + select { + case <-c: + case <-time.After(time.Second): + require.FailNow(t, "shutdown channel was not closed in time") + } +} + +// requireOpen asserts that the given channel is not yet closed. +func requireOpen(t *testing.T, c <-chan struct{}) { + t.Helper() + + select { + case <-c: + require.FailNow(t, "shutdown channel closed unexpectedly") + default: + } +} + +// TestShutdownSourceRequestShutdown checks that RequestShutdown closes the +// shutdown channel and that calling it again is a safe no-op. +func TestShutdownSourceRequestShutdown(t *testing.T) { + t.Parallel() + + s := newShutdownSource() + defer s.Stop() + + // The channel should start open. + requireOpen(t, s.ShutdownChannel()) + + // Requesting a shutdown should close it. + s.RequestShutdown() + waitClosed(t, s.ShutdownChannel()) + + // A second call must not panic on a double close. + require.NotPanics(t, s.RequestShutdown) + waitClosed(t, s.ShutdownChannel()) +} + +// TestShutdownSourceChannelStable checks that ShutdownChannel returns the same +// channel across calls, before and after a shutdown is requested, so callers +// can select on it repeatedly without spawning a goroutine per call. +func TestShutdownSourceChannelStable(t *testing.T) { + t.Parallel() + + s := newShutdownSource() + defer s.Stop() + + first := s.ShutdownChannel() + require.Equal(t, first, s.ShutdownChannel()) + + s.RequestShutdown() + + require.Equal(t, first, s.ShutdownChannel()) +} diff --git a/shutdown_unix_test.go b/shutdown_unix_test.go new file mode 100644 index 000000000..03256b3d1 --- /dev/null +++ b/shutdown_unix_test.go @@ -0,0 +1,29 @@ +//go:build !windows + +package terminal + +import ( + "syscall" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestShutdownSourceSignal checks that an OS termination signal closes the +// shutdown channel. The signal is captured by signal.Notify, so it does not +// terminate the test process. +// +// This lives in a Unix-only file because syscall.Kill is not available on +// Windows. +func TestShutdownSourceSignal(t *testing.T) { + // Not parallel: this sends a process-wide signal, which every + // registered shutdownSource would observe. + s := newShutdownSource() + defer s.Stop() + + requireOpen(t, s.ShutdownChannel()) + + require.NoError(t, syscall.Kill(syscall.Getpid(), syscall.SIGTERM)) + + waitClosed(t, s.ShutdownChannel()) +} diff --git a/terminal.go b/terminal.go index c2098bd6c..86550e946 100644 --- a/terminal.go +++ b/terminal.go @@ -191,6 +191,20 @@ type LightningTerminal struct { wg sync.WaitGroup errQueue *queue.ConcurrentQueue[error] + // shutdown is litd's own shutdown source. It is separate from the + // signal.Interceptor handed to lnd so that litd controls its own + // lifecycle and can keep running (to serve its status endpoint) after + // lnd alone has stopped. + shutdown *shutdownSource + + // lndStopped is set to true when lnd stops while litd keeps running. It + // lets the final shutdown skip the graceful lnd client teardown, which + // would otherwise block waiting on subscription goroutines that can no + // longer reach the now-dead lnd. On a clean shutdown it stays false so + // the lnd client is closed gracefully. It is only ever accessed from + // the main run goroutine. + lndStopped bool + lndConnID string lndConn *grpc.ClientConn lndClient *lndclient.GrpcLndServices @@ -269,31 +283,42 @@ func (s *stores) close() error { // Run starts everything and then blocks until either the application is shut // down or a critical error happens. func (g *LightningTerminal) Run(ctx context.Context) error { - // Hook interceptor for os signals. + // Hook interceptor for os signals. This interceptor is handed to lnd. + // We intentionally keep litd's own shutdown separate from it (see + // shutdownSource) so that lnd stopping does not force litd to stop. shutdownInterceptor, err := signal.Intercept() if err != nil { return fmt.Errorf("could not intercept signals: %v", err) } + // Create litd's own shutdown source. It listens for OS signals + // independently of lnd's interceptor and is passed to the loggers and + // the RPC proxy below so that a critical log or a litd StopDaemon call + // can bring litd down. + g.shutdown = newShutdownSource() + defer g.shutdown.Stop() + ctx, cancel := context.WithCancel(ctx) defer cancel() - // Make sure the context is canceled if the user requests shutdown and - // that the shutdown signal is requested if the context is canceled. + // Make sure the context is canceled if litd requests shutdown and that + // litd shutdown is requested if the context is canceled. Note that we + // watch litd's own shutdown source here, not lnd's interceptor, so that + // an lnd-only shutdown does not cancel litd's context. go func() { select { // Client requests shutdown, cancel the wait. - case <-shutdownInterceptor.ShutdownChannel(): + case <-g.shutdown.ShutdownChannel(): cancel() // The check was completed and the above defer canceled the // context. We can just exit the goroutine, nothing more to do. case <-ctx.Done(): - shutdownInterceptor.RequestShutdown() + g.shutdown.RequestShutdown() } }() - cfg, err := loadAndValidateConfig(ctx, shutdownInterceptor) + cfg, err := loadAndValidateConfig(ctx, shutdownInterceptor, g.shutdown) if err != nil { return fmt.Errorf("could not load config: %w", err) } @@ -377,7 +402,7 @@ func (g *LightningTerminal) Run(ctx context.Context) error { // server is started. g.rpcProxy = newRpcProxy( g.cfg, g, g.validateSuperMacaroon, g.permsMgr, g.subServerMgr, - g.statusMgr, g.basicLNDClient, + g.statusMgr, g.basicLNDClient, g.shutdown, ) // Register any gRPC services that should be served using LiT's @@ -401,10 +426,16 @@ func (g *LightningTerminal) Run(ctx context.Context) error { } } + // lndQuit is closed by the lnd.Main goroutine (inside start) when lnd + // stops. We create it in Run so that both start, during startup, and + // waitForShutdown, at runtime, can observe it without it living on the + // struct. + lndQuit := make(chan struct{}) + // Attempt to start Lit and all of its sub-servers. If an error is // returned, it means that either one of Lit's internal sub-servers // could not start or LND could not start or be connected to. - startErr := g.start(ctx) + startErr := g.start(ctx, lndQuit) if startErr != nil { g.statusMgr.SetErrored( subservers.LIT, "could not start Lit: %v", startErr, @@ -424,10 +455,22 @@ func (g *LightningTerminal) Run(ctx context.Context) error { } } - // Now block until we receive an error or the main shutdown - // signal. - <-shutdownInterceptor.ShutdownChannel() - log.Infof("Shutdown signal received") + // We keep litd running so its status endpoint stays reachable only if + // it got far enough that the rpcProxy started; before that, litcli stop + // cannot reach litd, so there would be no way to stop it. If the proxy + // did start, waitForShutdown blocks until litd itself is asked to shut + // down, tearing down the lnd-dependent sub-servers if lnd stops + // meanwhile. Otherwise we fall through and shut down fully, so a + // startup failure is loud and a supervisor can restart us. + if g.rpcProxy.hasStarted() { + g.waitForShutdown(ctx, lndQuit) + } + + // litd is going down, so bring lnd down with it. This is idempotent and + // a no-op if lnd already stopped (e.g. the shutdown was triggered by an + // OS signal that lnd's interceptor also received). It is required so + // that the lnd.Main goroutine returns and g.wg.Wait below can complete. + shutdownInterceptor.RequestShutdown() err = g.shutdownSubServers() if err != nil { @@ -443,8 +486,11 @@ func (g *LightningTerminal) Run(ctx context.Context) error { // LND errors are considered fatal and will result in an error being returned. // If any of the sub-servers managed by the subServerMgr error while starting // up, these are considered non-fatal and will not result in an error being -// returned. -func (g *LightningTerminal) start(ctx context.Context) error { +// returned. lndQuit is closed by start's lnd.Main goroutine when lnd stops, so +// that the caller can keep litd running while reporting lnd as errored. +func (g *LightningTerminal) start(ctx context.Context, + lndQuit chan struct{}) error { + var err error accountServiceErrCallback := func(err error) { @@ -524,9 +570,9 @@ func (g *LightningTerminal) start(ctx context.Context) error { readyChan = make(chan struct{}) bufReadyChan = make(chan struct{}) unlockChan = make(chan struct{}) - lndQuit = make(chan struct{}) macChan = make(chan []byte, 1) ) + if g.cfg.LndMode == ModeIntegrated { lisCfg := lnd.ListenerCfg{ RPCListeners: []*lnd.ListenerWithSignal{{ @@ -579,19 +625,33 @@ func (g *LightningTerminal) start(ctx context.Context) error { // function during the execution of this, as `g` is // referenced in the passed `implCfg` err := lnd.Main(g.cfg.Lnd, lisCfg, implCfg, interceptor) + + // Mark lnd as errored on the status server so the + // status endpoint reports why it stopped. On a real + // crash we also push the error into the shared error + // queue, the single source of runtime errors, so the + // startup-phase selects can abort before the other + // sub-servers come up. if e, ok := err.(*flags.Error); err != nil && (!ok || e.Type != flags.ErrHelp) { - errStr := fmt.Sprintf("Error running main "+ - "lnd: %v", err) - log.Errorf(errStr) + lndErr := fmt.Sprintf( + "Error running main lnd: %v", err, + ) + log.Errorf(lndErr) - g.statusMgr.SetErrored(subservers.LND, errStr) + g.statusMgr.SetErrored(subservers.LND, lndErr) g.errQueue.ChanIn() <- err - - return + } else { + g.statusMgr.SetErrored( + subservers.LND, "lnd has stopped", + ) } + // Signal that lnd has stopped, whether it exited + // cleanly or with an error, so start can either fail + // startup or tear down the lnd-dependent sub-servers + // while keeping litd (and its status endpoint) alive. close(lndQuit) }() } else { @@ -614,17 +674,12 @@ func (g *LightningTerminal) start(ctx context.Context) error { case <-readyChan: case err := <-g.errQueue.ChanOut(): - g.statusMgr.SetErrored( - subservers.LND, "error from errQueue channel", - ) - - return fmt.Errorf("could not start LND: %v", err) + return fmt.Errorf("error from subsystem: %w", err) case <-lndQuit: - g.statusMgr.SetErrored( - subservers.LND, "lndQuit channel closed", - ) - + // lnd has stopped during startup. The lnd.Main goroutine has + // already marked lnd errored on the status server, so we just + // abort startup here. return fmt.Errorf("LND has stopped") case <-ctx.Done(): @@ -707,22 +762,20 @@ func (g *LightningTerminal) start(ctx context.Context) error { } // waitForSignal is a helper closure that can be used to wait on the - // given channel for a signal while also being responsive to an error - // from the error Queue, LND quiting or the interceptor receiving a - // shutdown signal. + // given channel for a signal while also being responsive to LND + // quitting or the context being canceled. waitForSignal := func(c chan struct{}) error { select { case <-c: return nil case err := <-g.errQueue.ChanOut(): - return err + return fmt.Errorf("error from subsystem: %w", err) case <-lndQuit: - g.statusMgr.SetErrored( - subservers.LND, "lndQuit channel closed", - ) - + // lnd has stopped during startup. The lnd.Main + // goroutine has already marked lnd errored on the + // status server, so we just abort startup here. return fmt.Errorf("LND has stopped") case <-ctx.Done(): @@ -822,29 +875,69 @@ func (g *LightningTerminal) start(ctx context.Context) error { return fmt.Errorf("could not start litd sub-servers: %v", err) } - // We can now set the status of LiT as running. + // We can now set the status of LiT as running. litd has finished + // starting up, so we return and let Run block on waitForShutdown. g.statusMgr.SetRunning(subservers.LIT) - // Now block until we receive an error or the main shutdown signal. - select { - case err := <-g.errQueue.ChanOut(): - if err != nil { - return fmt.Errorf("received critical error from "+ - "subsystem, shutting down: %v", err) - } + return nil +} - case <-lndQuit: - g.statusMgr.SetErrored( - subservers.LND, "lndQuit channel closed", - ) +// waitForShutdown blocks until litd itself is told to shut down. Unlike +// during startup, an lnd stop here does not bring litd down: we mark lnd as +// errored and LiT as stopped, tear down the lnd-dependent sub-servers, and +// keep litd running so its status endpoint stays available to report what +// happened. Runtime sub-server errors are logged only. Only litd's own +// shutdown (OS signal, litcli stop) or the context being canceled stops +// litd. +func (g *LightningTerminal) waitForShutdown(ctx context.Context, + lndQuit chan struct{}) { - return fmt.Errorf("LND is not running") + for { + select { + case <-g.shutdown.ShutdownChannel(): + log.Infof("Shutdown signal received") - case <-ctx.Done(): - log.Infof("Shutdown signal received") - } + return - return nil + case <-ctx.Done(): + return + + case err := <-g.errQueue.ChanOut(): + // Once litd is up, a runtime sub-server error no + // longer brings it down: we keep litd (and its + // status endpoint) running so the failure stays + // observable. Such errors are commonly the fallout + // of lnd stopping (e.g. the RPC middleware losing + // its lnd connection); the lnd stop itself is + // handled via lndQuit below, so we only log here and + // keep waiting. + log.Errorf("Sub-server reported a runtime error; "+ + "keeping litd running so it stays observable: "+ + "%v", err) + + case <-lndQuit: + // lnd has stopped while litd was running. The + // lnd.Main goroutine has already set the LND + // sub-server status (the real error, or a generic + // message on a clean stop). Record that lnd is gone + // so the final shutdown skips the graceful lnd client + // teardown, mark LiT as errored so the status endpoint + // reports why it is no longer running, and tear down + // the lnd-dependent sub-servers while keeping litd + // running. + g.lndStopped = true + g.statusMgr.SetErrored( + subservers.LIT, "lit was stopped as lnd was "+ + "shut down", + ) + g.shutdownLndDependentSubServers() + + // Disable this case so the now-closed channel does not + // busy-loop the select while we wait for litd's own + // shutdown. + lndQuit = nil + } + } } // basicLNDClient provides access to LiT's basicClient if it has been set. @@ -864,7 +957,7 @@ func (g *LightningTerminal) checkRunning(ctx context.Context, select { case err := <-g.errQueue.ChanOut(): - return fmt.Errorf("error from subsystem: %v", err) + return fmt.Errorf("error from subsystem: %w", err) case <-lndQuit: return fmt.Errorf("LND has stopped") @@ -1538,6 +1631,47 @@ func (g *LightningTerminal) buildAuxComponents( }, nil } +// shutdownLndDependentSubServers stops the sub-servers and sub-systems that +// depend on lnd and cannot function once it has stopped, while leaving litd and +// its lnd-independent sub-servers (such as the status endpoint) running. +// +// It is called from waitForShutdown (which runs in Run) when lnd stops after +// litd has finished starting up, before the final shutdownSubServers, so it +// never races that full shutdown. Each underlying Stop is in any case +// idempotent. +func (g *LightningTerminal) shutdownLndDependentSubServers() { + log.Infof("lnd has stopped; shutting down lnd-dependent sub-servers " + + "while keeping litd's status endpoint available") + + // Stop the daemon sub-servers (loop, pool, tapd, faraday). + // Manager.Stop is idempotent and marks each stopped sub-server in the + // status manager. + if err := g.subServerMgr.Stop(); err != nil { + log.Errorf("Error stopping sub-servers after lnd stopped: %v", + err) + } + + // The autopilot client talks to lnd, so stop it too. Its Stop is + // guarded by a sync.Once and is safe to call again during the final + // shutdown. + if g.autopilotClient != nil { + g.autopilotClient.Stop() + } + + // Stop the internal accounts service, which also depends on lnd. + // Guard it so the final shutdownSubServers does not stop it a second + // time (its Stop closes a channel and is not idempotent). + if g.accountServiceStarted { + if err := g.accountService.Stop(); err != nil { + log.Errorf("Error stopping account service after lnd "+ + "stopped: %v", err) + } + g.accountServiceStarted = false + + g.statusMgr.SetStopped(subservers.ACCOUNTS) + } +} + // shutdownSubServers stops all subservers that were started and attached to // lnd. func (g *LightningTerminal) shutdownSubServers() error { @@ -1605,7 +1739,11 @@ func (g *LightningTerminal) shutdownSubServers() error { } } - if g.lndClient != nil { + // Skip the graceful lnd client teardown if lnd has already stopped: its + // Close waits on subscription goroutines that can no longer reach lnd + // and would block shutdown indefinitely. The process is exiting, so the + // connection does not need an orderly close in that case. + if g.lndClient != nil && !g.lndStopped { g.lndClient.Close() } @@ -1634,7 +1772,7 @@ func (g *LightningTerminal) shutdownSubServers() error { } } - // Do we have any last errors to display? We use an anonymous function, + // Drain and log any last runtime errors. We use an anonymous function // so we can use return instead of breaking to a label in the default // case. func() {