From cdaafc4a23a809684233999e87307aad6f13d0b0 Mon Sep 17 00:00:00 2001
From: "Johan T. Halseth" <johanth@gmail.com>
Date: Fri, 1 Jun 2018 09:18:55 +0200
Subject: [PATCH] fundingmanager: hold mutex during ctx cancellation

This commit changes cancelReservationCtx to gold the resMtx from start
to finish. Earlier it would lock at different times only when accessing
the maps, meaning that other goroutines (I'm looking at you
PeerTerminationWatcher) could come in and grab the context in between
locks, possibly leading to a race.
---
 fundingmanager.go | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/fundingmanager.go b/fundingmanager.go
index 485f5a96a..5039f2af5 100644
--- a/fundingmanager.go
+++ b/fundingmanager.go
@@ -2749,10 +2749,21 @@ func (f *fundingManager) cancelReservationCtx(peerKey *btcec.PublicKey,
 	fndgLog.Infof("Cancelling funding reservation for node_key=%x, "+
 		"chan_id=%x", peerKey.SerializeCompressed(), pendingChanID[:])
 
-	ctx, err := f.getReservationCtx(peerKey, pendingChanID)
-	if err != nil {
-		return nil, errors.Errorf("unable to find reservation: %v",
-			err)
+	peerIDKey := newSerializedKey(peerKey)
+	f.resMtx.Lock()
+	defer f.resMtx.Unlock()
+
+	nodeReservations, ok := f.activeReservations[peerIDKey]
+	if !ok {
+		// No reservations for this node.
+		return nil, errors.Errorf("no active reservations for peer(%x)",
+			peerIDKey[:])
+	}
+
+	ctx, ok := nodeReservations[pendingChanID]
+	if !ok {
+		return nil, errors.Errorf("unknown channel (id: %x) for "+
+			"peer(%x)", pendingChanID[:], peerIDKey[:])
 	}
 
 	if err := ctx.reservation.Cancel(); err != nil {
@@ -2760,7 +2771,13 @@ func (f *fundingManager) cancelReservationCtx(peerKey *btcec.PublicKey,
 			err)
 	}
 
-	f.deleteReservationCtx(peerKey, pendingChanID)
+	delete(nodeReservations, pendingChanID)
+
+	// If this was the last active reservation for this peer, delete the
+	// peer's entry altogether.
+	if len(nodeReservations) == 0 {
+		delete(f.activeReservations, peerIDKey)
+	}
 	return ctx, nil
 }
 
@@ -2800,8 +2817,8 @@ func (f *fundingManager) getReservationCtx(peerKey *btcec.PublicKey,
 	f.resMtx.RUnlock()
 
 	if !ok {
-		return nil, errors.Errorf("unknown channel (id: %x)",
-			pendingChanID[:])
+		return nil, errors.Errorf("unknown channel (id: %x) for "+
+			"peer(%x)", pendingChanID[:], peerIDKey[:])
 	}
 
 	return resCtx, nil