Skip to content

Commit

Permalink
multi: prevent nil panics in stop methods.
Browse files Browse the repository at this point in the history
With this PR we might call the stop method even when the start
method of a subsystem did not successfully finish therefore we
need to make sure we guard the stop methods for potential panics
if some variables are not initialized in the contructors of the
subsystems.
  • Loading branch information
ziggie1984 committed Jul 27, 2024
1 parent 662b7f7 commit 4bb3061
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 5 deletions.
7 changes: 6 additions & 1 deletion chainntnfs/bitcoindnotify/bitcoind.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ func (b *BitcoindNotifier) Stop() error {

close(epochClient.epochChan)
}
b.txNotifier.TearDown()

// The txNotifier is only initialized in the start method therefore we
// need to make sure we don't access a nil pointer here.
if b.txNotifier != nil {
b.txNotifier.TearDown()
}

// Stop the mempool notifier.
b.memNotifier.TearDown()
Expand Down
7 changes: 6 additions & 1 deletion chainntnfs/neutrinonotify/neutrino.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,12 @@ func (n *NeutrinoNotifier) Stop() error {

close(epochClient.epochChan)
}
n.txNotifier.TearDown()

// The txNotifier is only initialized in the start method therefore we
// need to make sure we don't access a nil pointer here.
if n.txNotifier != nil {
n.txNotifier.TearDown()
}

return nil
}
Expand Down
5 changes: 4 additions & 1 deletion chanfitness/chaneventstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,10 @@ func (c *ChannelEventStore) Stop() error {

// Stop the ticker after the goroutine reading from it has exited, to
// avoid a race.
c.cfg.FlapCountTicker.Stop()
if c.cfg.FlapCountTicker == nil {
return fmt.Errorf("ChannelEventStore FlapCountTicker not " +
"initialized")
}

log.Debugf("ChannelEventStore shutdown complete")

Expand Down
6 changes: 5 additions & 1 deletion discovery/gossiper.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,11 @@ func (d *AuthenticatedGossiper) stop() {
log.Debug("Authenticated Gossiper is stopping")
defer log.Debug("Authenticated Gossiper stopped")

d.blockEpochs.Cancel()
// `blockEpochs` is only initialized in the start routine so we make
// sure we don't panic here.
if d.blockEpochs != nil {
d.blockEpochs.Cancel()
}

d.syncMgr.Stop()

Expand Down
6 changes: 5 additions & 1 deletion htlcswitch/interceptable_switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,11 @@ func (s *InterceptableSwitch) Stop() error {
close(s.quit)
s.wg.Wait()

s.blockEpochStream.Cancel()
// We need to check whether the start routine run and initialized the
// `blockEpochStream`.
if s.blockEpochStream != nil {
s.blockEpochStream.Cancel()
}

log.Debug("InterceptableSwitch shutdown complete")

Expand Down
5 changes: 5 additions & 0 deletions invoices/invoiceregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,11 @@ func (i *InvoiceRegistry) Stop() error {
log.Info("InvoiceRegistry shutting down...")
defer log.Debug("InvoiceRegistry shutdown complete")

if i.expiryWatcher == nil {
return fmt.Errorf("InvoiceRegistry expiryWatcher not " +
"initialized")
}

i.expiryWatcher.Stop()

close(i.quit)
Expand Down
3 changes: 3 additions & 0 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2107,6 +2107,9 @@ func (s *server) Start() error {
}
}

// chanSubSwapper must be started after the `channelNotifier`
// because it depends on channel events as a synchronization
// point.
cleanup = cleanup.add(s.chanSubSwapper.Stop)
if err := s.chanSubSwapper.Start(); err != nil {
startErr = err
Expand Down

0 comments on commit 4bb3061

Please sign in to comment.