From 3223df74e5972c437bfaf18d3e84aa1e0b47b4c1 Mon Sep 17 00:00:00 2001 From: Roei Erez Date: Mon, 21 Jan 2019 13:11:19 +0200 Subject: [PATCH] channelnotifier+discover+invoices: return error in Stop functions In order to be consistent with other sub systems an error is now returned from the Stop functions. This also allows writing a generic cleanup mechanism to stop all sub systems in case of a failure. --- channelnotifier/channelnotifier.go | 6 ++++-- discovery/gossiper.go | 3 ++- funding/manager.go | 4 +++- funding/manager_test.go | 16 ++++++++++++---- htlcswitch/htlcnotifier.go | 6 ++++-- htlcswitch/switch_test.go | 18 +++++++++++++++--- invoices/invoiceregistry.go | 5 +++-- invoices/invoiceregistry_test.go | 10 ++++++++-- invoices/test_utils_test.go | 4 +++- peernotifier/peernotifier.go | 6 ++++-- 10 files changed, 58 insertions(+), 20 deletions(-) diff --git a/channelnotifier/channelnotifier.go b/channelnotifier/channelnotifier.go index 5d67fd51..f5aa2961 100644 --- a/channelnotifier/channelnotifier.go +++ b/channelnotifier/channelnotifier.go @@ -86,10 +86,12 @@ func (c *ChannelNotifier) Start() error { } // Stop signals the notifier for a graceful shutdown. -func (c *ChannelNotifier) Stop() { +func (c *ChannelNotifier) Stop() error { + var err error c.stopped.Do(func() { - c.ntfnServer.Stop() + err = c.ntfnServer.Stop() }) + return err } // SubscribeChannelEvents returns a subscribe.Client that will receive updates diff --git a/discovery/gossiper.go b/discovery/gossiper.go index 2c4c07b0..5b6273d7 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -461,8 +461,9 @@ func (d *AuthenticatedGossiper) start() error { } // Stop signals any active goroutines for a graceful closure. -func (d *AuthenticatedGossiper) Stop() { +func (d *AuthenticatedGossiper) Stop() error { d.stopped.Do(d.stop) + return nil } func (d *AuthenticatedGossiper) stop() { diff --git a/funding/manager.go b/funding/manager.go index a02ae114..f74b5c99 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -675,12 +675,14 @@ func (f *Manager) start() error { // Stop signals all helper goroutines to execute a graceful shutdown. This // method will block until all goroutines have exited. -func (f *Manager) Stop() { +func (f *Manager) Stop() error { f.stopped.Do(func() { log.Info("Funding manager shutting down") close(f.quit) f.wg.Wait() }) + + return nil } // nextPendingChanID returns the next free pending channel ID to be used to diff --git a/funding/manager_test.go b/funding/manager_test.go index 4a995040..fa08d2e0 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -484,7 +484,9 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, func recreateAliceFundingManager(t *testing.T, alice *testNode) { // Stop the old fundingManager before creating a new one. close(alice.shutdownChannel) - alice.fundingMgr.Stop() + if err := alice.fundingMgr.Stop(); err != nil { + t.Fatalf("failed stop funding manager: %v", err) + } aliceMsgChan := make(chan lnwire.Message) aliceAnnounceChan := make(chan lnwire.Message) @@ -622,8 +624,12 @@ func tearDownFundingManagers(t *testing.T, a, b *testNode) { close(a.shutdownChannel) close(b.shutdownChannel) - a.fundingMgr.Stop() - b.fundingMgr.Stop() + if err := a.fundingMgr.Stop(); err != nil { + t.Fatalf("failed stop funding manager: %v", err) + } + if err := b.fundingMgr.Stop(); err != nil { + t.Fatalf("failed stop funding manager: %v", err) + } os.RemoveAll(a.testDir) os.RemoveAll(b.testDir) } @@ -1502,7 +1508,9 @@ func TestFundingManagerRestartBehavior(t *testing.T) { // implementation, and expect it to retry sending the fundingLocked // message. We'll explicitly shut down Alice's funding manager to // prevent a race when overriding the sendMessage implementation. - alice.fundingMgr.Stop() + if err := alice.fundingMgr.Stop(); err != nil { + t.Fatalf("failed stop funding manager: %v", err) + } bob.sendMessage = workingSendMessage recreateAliceFundingManager(t, alice) diff --git a/htlcswitch/htlcnotifier.go b/htlcswitch/htlcnotifier.go index 25953e65..e73ec543 100644 --- a/htlcswitch/htlcnotifier.go +++ b/htlcswitch/htlcnotifier.go @@ -87,12 +87,14 @@ func (h *HtlcNotifier) Start() error { } // Stop signals the notifier for a graceful shutdown. -func (h *HtlcNotifier) Stop() { +func (h *HtlcNotifier) Stop() error { + var err error h.stopped.Do(func() { - if err := h.ntfnServer.Stop(); err != nil { + if err = h.ntfnServer.Stop(); err != nil { log.Warnf("error stopping htlc notifier: %v", err) } }) + return err } // SubscribeHtlcEvents returns a subscribe.Client that will receive updates diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index fac7a9dd..adf18d91 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -2867,19 +2867,31 @@ func testHtcNotifier(t *testing.T, testOpts []serverOption, iterations int, if err := aliceNotifier.Start(); err != nil { t.Fatalf("could not start alice notifier") } - defer aliceNotifier.Stop() + defer func() { + if err := aliceNotifier.Stop(); err != nil { + t.Fatalf("failed to stop alice notifier: %v", err) + } + }() bobNotifier := NewHtlcNotifier(mockTime) if err := bobNotifier.Start(); err != nil { t.Fatalf("could not start bob notifier") } - defer bobNotifier.Stop() + defer func() { + if err := bobNotifier.Stop(); err != nil { + t.Fatalf("failed to stop bob notifier: %v", err) + } + }() carolNotifier := NewHtlcNotifier(mockTime) if err := carolNotifier.Start(); err != nil { t.Fatalf("could not start carol notifier") } - defer carolNotifier.Stop() + defer func() { + if err := carolNotifier.Stop(); err != nil { + t.Fatalf("failed to stop carol notifier: %v", err) + } + }() // Create a notifier server option which will set our htlc notifiers // for the three hop network. diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index b73de5e5..bb336be7 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -237,7 +237,7 @@ func (i *InvoiceRegistry) Start() error { // delete them. err = i.scanInvoicesOnStart() if err != nil { - i.Stop() + _ = i.Stop() return err } @@ -245,12 +245,13 @@ func (i *InvoiceRegistry) Start() error { } // Stop signals the registry for a graceful shutdown. -func (i *InvoiceRegistry) Stop() { +func (i *InvoiceRegistry) Stop() error { i.expiryWatcher.Stop() close(i.quit) i.wg.Wait() + return nil } // invoiceEvent represents a new event that has modified on invoice on disk. diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 1d1198eb..799d5395 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -527,7 +527,11 @@ func TestCancelHoldInvoice(t *testing.T) { if err != nil { t.Fatal(err) } - defer registry.Stop() + defer func() { + if err := registry.Stop(); err != nil { + t.Fatalf("failed to stop invoice registry: %v", err) + } + }() // Add the invoice. _, err = registry.AddInvoice(testHodlInvoice, testInvoicePaymentHash) @@ -1005,7 +1009,9 @@ func TestInvoiceExpiryWithRegistry(t *testing.T) { // Give some time to the watcher to cancel everything. time.Sleep(500 * time.Millisecond) - registry.Stop() + if err = registry.Stop(); err != nil { + t.Fatalf("failed to stop invoice registry: %v", err) + } // Create the expected cancellation set before the final check. expectedCancellations = append( diff --git a/invoices/test_utils_test.go b/invoices/test_utils_test.go index 6a454a9e..3e49a957 100644 --- a/invoices/test_utils_test.go +++ b/invoices/test_utils_test.go @@ -215,7 +215,9 @@ func newTestContext(t *testing.T) *testContext { clock: clock, t: t, cleanup: func() { - registry.Stop() + if err = registry.Stop(); err != nil { + t.Fatalf("failed to stop invoice registry: %v", err) + } cleanup() }, } diff --git a/peernotifier/peernotifier.go b/peernotifier/peernotifier.go index 0943c82a..4a92ff26 100644 --- a/peernotifier/peernotifier.go +++ b/peernotifier/peernotifier.go @@ -49,11 +49,13 @@ func (p *PeerNotifier) Start() error { } // Stop signals the notifier for a graceful shutdown. -func (p *PeerNotifier) Stop() { +func (p *PeerNotifier) Stop() error { + var err error p.stopped.Do(func() { log.Info("Stopping PeerNotifier") - p.ntfnServer.Stop() + err = p.ntfnServer.Stop() }) + return err } // SubscribePeerEvents returns a subscribe.Client that will receive updates