Merge pull request #4226 from breez/fix-chainrpc-crash

Fix crash when ChainNotifier is being accessed too early.

chainntnfs/bitcoindnotify/bitcoind.go

@@ -34,7 +34,8 @@ const (
 type BitcoindNotifier struct {
 	epochClientCounter uint64 // To be used atomically.
 
-	started int32 // To be used atomically.
+	start   sync.Once
+	active  int32 // To be used atomically.
 	stopped int32 // To be used atomically.
 
 	chainConn   *chain.BitcoindClient
@@ -96,11 +97,46 @@ func New(chainConn *chain.BitcoindConn, chainParams *chaincfg.Params,
 // Start connects to the running bitcoind node over websockets, registers for
 // block notifications, and finally launches all related helper goroutines.
 func (b *BitcoindNotifier) Start() error {
-	// Already started?
-	if atomic.AddInt32(&b.started, 1) != 1 {
+	var startErr error
+	b.start.Do(func() {
+		startErr = b.startNotifier()
+	})
+	return startErr
+}
+
+// Stop shutsdown the BitcoindNotifier.
+func (b *BitcoindNotifier) Stop() error {
+	// Already shutting down?
+	if atomic.AddInt32(&b.stopped, 1) != 1 {
 		return nil
 	}
 
+	// Shutdown the rpc client, this gracefully disconnects from bitcoind,
+	// and cleans up all related resources.
+	b.chainConn.Stop()
+
+	close(b.quit)
+	b.wg.Wait()
+
+	// Notify all pending clients of our shutdown by closing the related
+	// notification channels.
+	for _, epochClient := range b.blockEpochClients {
+		close(epochClient.cancelChan)
+		epochClient.wg.Wait()
+
+		close(epochClient.epochChan)
+	}
+	b.txNotifier.TearDown()
+
+	return nil
+}
+
+// Started returns true if this instance has been started, and false otherwise.
+func (b *BitcoindNotifier) Started() bool {
+	return atomic.LoadInt32(&b.active) != 0
+}
+
+func (b *BitcoindNotifier) startNotifier() error {
 	// Connect to bitcoind, and register for notifications on connected,
 	// and disconnected blocks.
 	if err := b.chainConn.Start(); err != nil {
@@ -128,32 +164,9 @@ func (b *BitcoindNotifier) Start() error {
 	b.wg.Add(1)
 	go b.notificationDispatcher()
 
-	return nil
-}
-
-// Stop shutsdown the BitcoindNotifier.
-func (b *BitcoindNotifier) Stop() error {
-	// Already shutting down?
-	if atomic.AddInt32(&b.stopped, 1) != 1 {
-		return nil
-	}
-
-	// Shutdown the rpc client, this gracefully disconnects from bitcoind,
-	// and cleans up all related resources.
-	b.chainConn.Stop()
-
-	close(b.quit)
-	b.wg.Wait()
-
-	// Notify all pending clients of our shutdown by closing the related
-	// notification channels.
-	for _, epochClient := range b.blockEpochClients {
-		close(epochClient.cancelChan)
-		epochClient.wg.Wait()
-
-		close(epochClient.epochChan)
-	}
-	b.txNotifier.TearDown()
+	// Set the active flag now that we've completed the full
+	// startup.
+	atomic.StoreInt32(&b.active, 1)
 
 	return nil
 }

chainntnfs/btcdnotify/btcd.go

@@ -53,7 +53,8 @@ type txUpdate struct {
 type BtcdNotifier struct {
 	epochClientCounter uint64 // To be used atomically.
 
-	started int32 // To be used atomically.
+	start   sync.Once
+	active  int32 // To be used atomically.
 	stopped int32 // To be used atomically.
 
 	chainConn   *rpcclient.Client
@@ -134,11 +135,49 @@ func New(config *rpcclient.ConnConfig, chainParams *chaincfg.Params,
 // Start connects to the running btcd node over websockets, registers for block
 // notifications, and finally launches all related helper goroutines.
 func (b *BtcdNotifier) Start() error {
-	// Already started?
-	if atomic.AddInt32(&b.started, 1) != 1 {
+	var startErr error
+	b.start.Do(func() {
+		startErr = b.startNotifier()
+	})
+	return startErr
+}
+
+// Started returns true if this instance has been started, and false otherwise.
+func (b *BtcdNotifier) Started() bool {
+	return atomic.LoadInt32(&b.active) != 0
+}
+
+// Stop shutsdown the BtcdNotifier.
+func (b *BtcdNotifier) Stop() error {
+	// Already shutting down?
+	if atomic.AddInt32(&b.stopped, 1) != 1 {
 		return nil
 	}
 
+	// Shutdown the rpc client, this gracefully disconnects from btcd, and
+	// cleans up all related resources.
+	b.chainConn.Shutdown()
+
+	close(b.quit)
+	b.wg.Wait()
+
+	b.chainUpdates.Stop()
+	b.txUpdates.Stop()
+
+	// Notify all pending clients of our shutdown by closing the related
+	// notification channels.
+	for _, epochClient := range b.blockEpochClients {
+		close(epochClient.cancelChan)
+		epochClient.wg.Wait()
+
+		close(epochClient.epochChan)
+	}
+	b.txNotifier.TearDown()
+
+	return nil
+}
+
+func (b *BtcdNotifier) startNotifier() error {
 	// Start our concurrent queues before starting the chain connection, to
 	// ensure onBlockConnected and onRedeemingTx callbacks won't be
 	// blocked.
@@ -179,35 +218,9 @@ func (b *BtcdNotifier) Start() error {
 	b.wg.Add(1)
 	go b.notificationDispatcher()
 
-	return nil
-}
-
-// Stop shutsdown the BtcdNotifier.
-func (b *BtcdNotifier) Stop() error {
-	// Already shutting down?
-	if atomic.AddInt32(&b.stopped, 1) != 1 {
-		return nil
-	}
-
-	// Shutdown the rpc client, this gracefully disconnects from btcd, and
-	// cleans up all related resources.
-	b.chainConn.Shutdown()
-
-	close(b.quit)
-	b.wg.Wait()
-
-	b.chainUpdates.Stop()
-	b.txUpdates.Stop()
-
-	// Notify all pending clients of our shutdown by closing the related
-	// notification channels.
-	for _, epochClient := range b.blockEpochClients {
-		close(epochClient.cancelChan)
-		epochClient.wg.Wait()
-
-		close(epochClient.epochChan)
-	}
-	b.txNotifier.TearDown()
+	// Set the active flag now that we've completed the full
+	// startup.
+	atomic.StoreInt32(&b.active, 1)
 
 	return nil
 }

chainntnfs/interface.go

@@ -139,6 +139,9 @@ type ChainNotifier interface {
 	// ready, and able to receive notification registrations from clients.
 	Start() error
 
+	// Started returns true if this instance has been started, and false otherwise.
+	Started() bool
+
 	// Stops the concrete ChainNotifier. Once stopped, the ChainNotifier
 	// should disallow any future requests from potential clients.
 	// Additionally, all pending client notifications will be canceled

chainntnfs/neutrinonotify/neutrino.go

@@ -39,7 +39,8 @@ const (
 type NeutrinoNotifier struct {
 	epochClientCounter uint64 // To be used atomically.
 
-	started int32 // To be used atomically.
+	start   sync.Once
+	active  int32 // To be used atomically.
 	stopped int32 // To be used atomically.
 
 	bestBlockMtx sync.RWMutex
@@ -111,11 +112,45 @@ func New(node *neutrino.ChainService, spendHintCache chainntnfs.SpendHintCache,
 // Start contacts the running neutrino light client and kicks off an initial
 // empty rescan.
 func (n *NeutrinoNotifier) Start() error {
-	// Already started?
-	if atomic.AddInt32(&n.started, 1) != 1 {
+	var startErr error
+	n.start.Do(func() {
+		startErr = n.startNotifier()
+	})
+	return startErr
+}
+
+// Stop shuts down the NeutrinoNotifier.
+func (n *NeutrinoNotifier) Stop() error {
+	// Already shutting down?
+	if atomic.AddInt32(&n.stopped, 1) != 1 {
 		return nil
 	}
 
+	close(n.quit)
+	n.wg.Wait()
+
+	n.chainUpdates.Stop()
+	n.txUpdates.Stop()
+
+	// Notify all pending clients of our shutdown by closing the related
+	// notification channels.
+	for _, epochClient := range n.blockEpochClients {
+		close(epochClient.cancelChan)
+		epochClient.wg.Wait()
+
+		close(epochClient.epochChan)
+	}
+	n.txNotifier.TearDown()
+
+	return nil
+}
+
+// Started returns true if this instance has been started, and false otherwise.
+func (n *NeutrinoNotifier) Started() bool {
+	return atomic.LoadInt32(&n.active) != 0
+}
+
+func (n *NeutrinoNotifier) startNotifier() error {
 	// Start our concurrent queues before starting the rescan, to ensure
 	// onFilteredBlockConnected and onRelavantTx callbacks won't be
 	// blocked.
@@ -171,31 +206,9 @@ func (n *NeutrinoNotifier) Start() error {
 	n.wg.Add(1)
 	go n.notificationDispatcher()
 
-	return nil
-}
-
-// Stop shuts down the NeutrinoNotifier.
-func (n *NeutrinoNotifier) Stop() error {
-	// Already shutting down?
-	if atomic.AddInt32(&n.stopped, 1) != 1 {
-		return nil
-	}
-
-	close(n.quit)
-	n.wg.Wait()
-
-	n.chainUpdates.Stop()
-	n.txUpdates.Stop()
-
-	// Notify all pending clients of our shutdown by closing the related
-	// notification channels.
-	for _, epochClient := range n.blockEpochClients {
-		close(epochClient.cancelChan)
-		epochClient.wg.Wait()
-
-		close(epochClient.epochChan)
-	}
-	n.txNotifier.TearDown()
+	// Set the active flag now that we've completed the full
+	// startup.
+	atomic.StoreInt32(&n.active, 1)
 
 	return nil
 }