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

@@ -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) {

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)
+}