From e7dc2f91900c9394529190de921c96b3b2d4eb44 Mon Sep 17 00:00:00 2001
From: Graham Miln <graham.miln@dssw.co.uk>
Date: Wed, 21 Mar 2018 08:02:32 +0100
Subject: [PATCH] lib/nat: Fix clearAddresses/notify deadlock (ref #4601)
 (#4829)

clearAddresses write locks the struct and then calls notify. notify in turn tries to obtain a read lock on the same mutex. The result was a deadlock. This change unlocks the struct before calling notify.
---
 lib/nat/structs.go      |  4 ++--
 lib/nat/structs_test.go | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/nat/structs.go b/lib/nat/structs.go
index 2d218ca04..32dae4a5f 100644
--- a/lib/nat/structs.go
+++ b/lib/nat/structs.go
@@ -53,11 +53,11 @@ func (m *Mapping) clearAddresses() {
 		removed = append(removed, addr)
 		delete(m.extAddresses, id)
 	}
+	m.expires = time.Time{}
+	m.mut.Unlock()
 	if len(removed) > 0 {
 		m.notify(nil, removed)
 	}
-	m.expires = time.Time{}
-	m.mut.Unlock()
 }
 
 func (m *Mapping) notify(added, removed []Address) {
diff --git a/lib/nat/structs_test.go b/lib/nat/structs_test.go
index 49cd470c6..502387b20 100644
--- a/lib/nat/structs_test.go
+++ b/lib/nat/structs_test.go
@@ -9,6 +9,8 @@ package nat
 import (
 	"net"
 	"testing"
+
+	"github.com/syncthing/syncthing/lib/protocol"
 )
 
 func TestMappingValidGateway(t *testing.T) {
@@ -52,3 +54,16 @@ func TestMappingValidGateway(t *testing.T) {
 		}
 	}
 }
+
+func TestMappingClearAddresses(t *testing.T) {
+	natSvc := NewService(protocol.EmptyDeviceID, nil)
+	// Mock a mapped port; avoids the need to actually map a port
+	ip := net.ParseIP("192.168.0.1")
+	m := natSvc.NewMapping(TCP, ip, 1024)
+	m.extAddresses["test"] = Address{
+		IP:   ip,
+		Port: 1024,
+	}
+	// Now try and remove the mapped port; prior to #4829 this deadlocked
+	natSvc.RemoveMapping(m)
+}