From 62bcd59db4a9d81665ef95c835ae7342c924b5e0 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Thu, 2 Feb 2017 17:10:57 -0800
Subject: [PATCH] lnwallet: add ability to cancel the channel's closeObserver
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit adds the ability to cancel a channel’s internal
closeObserver goroutine by adding a new public facing Stop method.

Additionally, we now make passing a ChainNotifier interface completely
optional. If the ChainNotifier isn’t passed in as a constructor, then
the closeObserver goroutine will never be launched. This new feature
lets the caller ensure that only a single closeObsever for any given
channels exists.
---
 lnwallet/channel.go | 65 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 18 deletions(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index b56af1a13..54d42b528 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -441,11 +441,8 @@ type LightningChannel struct {
 	// channel.
 	RemoteFundingKey *btcec.PublicKey
 
-	started  int32
 	shutdown int32
-
-	quit chan struct{}
-	wg   sync.WaitGroup
+	quit     chan struct{}
 }
 
 // NewLightningChannel creates a new, active payment channel given an
@@ -477,6 +474,7 @@ func NewLightningChannel(signer Signer, events chainntnfs.ChainNotifier,
 		ContractBreach:        make(chan *BreachRetribution, 1),
 		LocalFundingKey:       state.OurMultiSigKey,
 		RemoteFundingKey:      state.TheirMultiSigKey,
+		quit:                  make(chan struct{}),
 	}
 
 	// Initialize both of our chains the current un-revoked commitment for
@@ -519,19 +517,25 @@ func NewLightningChannel(signer Signer, events chainntnfs.ChainNotifier,
 		InputIndex: 0,
 	}
 
-	// Register for a notification to be dispatched if the funding outpoint
-	// has been spent. This indicates that either us or the remote party
-	// has broadcasted a commitment transaction on-chain.
-	fundingOut := &lc.fundingTxIn.PreviousOutPoint
-	channelCloseNtfn, err := lc.channelEvents.RegisterSpendNtfn(fundingOut)
-	if err != nil {
-		return nil, err
-	}
+	// We'll only launch a close observer if the ChainNotifier
+	// implementation is non-nil. Passing a nil value indicates that the
+	// channel shouldn't be actively watched for.
+	if lc.channelEvents != nil {
+		// Register for a notification to be dispatched if the funding
+		// outpoint has been spent. This indicates that either us or
+		// the remote party has broadcasted a commitment transaction
+		// on-chain.
+		fundingOut := &lc.fundingTxIn.PreviousOutPoint
+		channelCloseNtfn, err := lc.channelEvents.RegisterSpendNtfn(fundingOut)
+		if err != nil {
+			return nil, err
+		}
 
-	// Launch the close observer which will vigilantly watch the network
-	// for any broadcasts the current or prior commitment transactions,
-	// taking action accordingly.
-	go lc.closeObserver(channelCloseNtfn)
+		// Launch the close observer which will vigilantly watch the
+		// network for any broadcasts the current or prior commitment
+		// transactions, taking action accordingly.
+		go lc.closeObserver(channelCloseNtfn)
+	}
 
 	return lc, nil
 }
@@ -682,10 +686,25 @@ func newBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64,
 //
 // NOTE: This MUST be run as a goroutine.
 func (lc *LightningChannel) closeObserver(channelCloseNtfn *chainntnfs.SpendEvent) {
+	walletLog.Infof("Close observer for ChannelPoint(%v) active",
+		lc.channelState.ChanID)
+
+	var (
+		commitSpend *chainntnfs.SpendDetail
+		ok          bool
+	)
+
+	select {
 	// If the daemon is shutting down, then this notification channel will
 	// be closed, so check the second read-value to avoid a false positive.
-	commitSpend, ok := <-channelCloseNtfn.Spend
-	if !ok {
+	case commitSpend, ok = <-channelCloseNtfn.Spend:
+		if !ok {
+			return
+		}
+
+	// Otherwise, we've be signalled to bail out early by the
+	// caller/maintainer of this channel.
+	case <-lc.quit:
 		return
 	}
 
@@ -769,6 +788,16 @@ func (lc *LightningChannel) closeObserver(channelCloseNtfn *chainntnfs.SpendEven
 	}
 }
 
+// Stop gracefully shuts down any active goroutines spawned by the
+// LightningChannel during regular duties.
+func (lc *LightningChannel) Stop() {
+	if !atomic.CompareAndSwapInt32(&lc.shutdown, 0, 1) {
+		return
+	}
+
+	close(lc.quit)
+}
+
 // restoreStateLogs runs through the current locked-in HTLCs from the point of
 // view of the channel and insert corresponding log entries (both local and
 // remote) for each HTLC read from disk. This method is required sync the