From 4243c5b95fa9d204267684819b897c11bce81f95 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 19 Apr 2023 17:06:09 +0800 Subject: [PATCH 1/3] lnd: properly shutdown the optional pprof server This commit makes sure when lnd is shutting down, the optional pprof server is also gracefully shutting down. --- lnd.go | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/lnd.go b/lnd.go index 8da903e96..cb2d8790c 100644 --- a/lnd.go +++ b/lnd.go @@ -180,14 +180,42 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, network, ) + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + // Enable http profiling server if requested. if cfg.Profile != "" { + // Create the http handler. + profileRedirect := http.RedirectHandler( + "/debug/pprof", http.StatusSeeOther, + ) + http.Handle("/", profileRedirect) + ltndLog.Infof("Pprof listening on %v", cfg.Profile) + + // Create the pprof server. + pprofServer := &http.Server{ + Addr: cfg.Profile, + Handler: profileRedirect, + ReadHeaderTimeout: 5 * time.Second, + } + + // Shut the server down when lnd is shutting down. + defer func() { + ltndLog.Info("Stopping pprof server...") + err := pprofServer.Shutdown(ctx) + if err != nil { + ltndLog.Errorf("Stop pprof server got err: %v", + err) + } + }() + + // Start the pprof server. go func() { - profileRedirect := http.RedirectHandler("/debug/pprof", - http.StatusSeeOther) - http.Handle("/", profileRedirect) - ltndLog.Infof("Pprof listening on %v", cfg.Profile) - fmt.Println(http.ListenAndServe(cfg.Profile, nil)) + err := pprofServer.ListenAndServe() + if err != nil && !errors.Is(err, http.ErrServerClosed) { + ltndLog.Errorf("Serving pprof got err: %v", err) + } }() } @@ -202,10 +230,6 @@ func Main(cfg *Config, lisCfg ListenerCfg, implCfg *ImplementationCfg, defer pprof.StopCPUProfile() } - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) - defer cancel() - // Run configuration dependent DB pre-initialization. Note that this // needs to be done early and once during the startup process, before // any DB access. From 24c028ff52e9dcd4a7414b4c079faf1efe37a121 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 19 Apr 2023 17:24:25 +0800 Subject: [PATCH 2/3] lntest: make sure error from shutdown is caught --- lntest/node/harness_node.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lntest/node/harness_node.go b/lntest/node/harness_node.go index 0d8d97a14..ed4f324ae 100644 --- a/lntest/node/harness_node.go +++ b/lntest/node/harness_node.go @@ -6,6 +6,7 @@ import ( "crypto/rand" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -627,10 +628,12 @@ func (hn *HarnessNode) cleanup() error { // waitForProcessExit Launch a new goroutine which that bubbles up any // potential fatal process errors to the goroutine running the tests. -func (hn *HarnessNode) waitForProcessExit() { +func (hn *HarnessNode) waitForProcessExit() error { + var err error + errChan := make(chan error, 1) go func() { - err := hn.cmd.Wait() + err = hn.cmd.Wait() errChan <- err }() @@ -643,7 +646,7 @@ func (hn *HarnessNode) waitForProcessExit() { // If the process has already been canceled, we can exit early // as the logs have already been saved. if strings.Contains(err.Error(), "Wait was already called") { - return + return nil } // Otherwise, we print the error, break the select and save @@ -653,7 +656,8 @@ func (hn *HarnessNode) waitForProcessExit() { break case <-time.After(wait.DefaultTimeout): - hn.printErrf("timeout waiting for process to exit") + err = errors.New("timeout waiting for process to exit") + hn.printErrf(err.Error()) } // Make sure log file is closed and renamed if necessary. @@ -662,6 +666,8 @@ func (hn *HarnessNode) waitForProcessExit() { // Rename the etcd.log file if the node was running on embedded // etcd. finalizeEtcdLog(hn) + + return err } // Stop attempts to stop the active lnd process. @@ -675,9 +681,6 @@ func (hn *HarnessNode) Stop() error { // Stop the runCtx. hn.cancel() - // Wait for lnd process to exit in the end. - defer hn.waitForProcessExit() - // If we ever reaches the state where `Watcher` is initialized, it // means the node has an authed connection and all its RPC clients are // ready for use. Thus we will try to stop it via the RPC. @@ -740,7 +743,8 @@ func (hn *HarnessNode) Stop() error { } } - return nil + // Wait for lnd process to exit in the end. + return hn.waitForProcessExit() } // CloseConn closes the grpc connection. From e547118de0645be7855606ac0d09da70112a4fbb Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 20 Apr 2023 06:04:16 +0800 Subject: [PATCH 3/3] lntest: increase channel close timeout --- lntest/wait/timeouts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lntest/wait/timeouts.go b/lntest/wait/timeouts.go index f8239799b..21c99959b 100644 --- a/lntest/wait/timeouts.go +++ b/lntest/wait/timeouts.go @@ -16,7 +16,7 @@ const ( // ChannelCloseTimeout is the max time we will wait before a channel is // considered closed. - ChannelCloseTimeout = time.Second * 30 + ChannelCloseTimeout = time.Second * 60 // DefaultTimeout is a timeout that will be used for various wait // scenarios where no custom timeout value is defined.