From 97e730cf51f0a89c59750856f64d7373296b54c2 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 25 Sep 2017 15:59:23 -0700 Subject: [PATCH] htlcswitch: fix possible deadlock bug in packetQueue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a possible deadlock within the packetQueue that could be caused by the following circular waiting dependency: packetCoordinator woken up, grabs lock, queue isn’t empty, attempts to send packet to link (lock still held) -> channelLink has commitment overflow, attempts to add new item to packet queue, in AddPkt grabs Lock -> circular wait. We avoid this scenario by *not* holding the lock within the packetCoordinator when we attempt to send a new packet to the switch. Instead, we release the lock before the second select statement in the main processing loop. --- htlcswitch/queue.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/htlcswitch/queue.go b/htlcswitch/queue.go index 262824edf..83d6eb6c5 100644 --- a/htlcswitch/queue.go +++ b/htlcswitch/queue.go @@ -107,12 +107,15 @@ func (p *packetQueue) packetCoordinator() { nextPkt := p.queue[0] + p.queueCond.L.Unlock() + // If there aren't any further messages to sent (or the link // didn't immediately read our message), then we'll block and // wait for a new message to be sent into the overflow queue, // or for the link's htlcForwarder to wake up. select { case <-p.freeSlots: + select { case p.outgoingPkts <- nextPkt: // Pop the item off the front of the queue and @@ -120,22 +123,20 @@ func (p *packetQueue) packetCoordinator() { // the head pointer. This will set us up for // the next iteration. If the queue is empty // at this point, then we'll block at the top. + p.queueCond.L.Lock() p.queue[0] = nil p.queue = p.queue[1:] + p.queueCond.L.Unlock() atomic.AddInt32(&p.queueLen, -1) case <-p.quit: - p.queueCond.L.Unlock() return } case <-p.quit: - p.queueCond.L.Unlock() return default: } - - p.queueCond.L.Unlock() } }