From 3cd199e8523c19e306ce2474dfaa20b12db0045c Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Tue, 17 Nov 2015 20:13:46 +0000
Subject: [PATCH] make GossipData immutable

The same piece of GossipData can be shared and subject to different
Merge operations. We either need to copy the data when we share it, or
create copies on modification, i.e. copy-on-write.

We choose the latter, since it is more efficient and there is just one
mutating operation.

Fixes #1697.
---
 ipam/allocator.go          |  4 ++--
 mesh/gossip.go             |  5 ++---
 mesh/router.go             | 18 ++++++++++--------
 mesh/surrogate_gossiper.go |  8 ++++++--
 nameserver/entry.go        | 14 ++++++++++----
 nameserver/entry_test.go   |  4 ++--
 6 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/ipam/allocator.go b/ipam/allocator.go
index 888b9433..a98697c8 100644
--- a/ipam/allocator.go
+++ b/ipam/allocator.go
@@ -478,8 +478,8 @@ type ipamGossipData struct {
 	alloc *Allocator
 }
 
-func (d *ipamGossipData) Merge(other mesh.GossipData) {
-	// no-op
+func (d *ipamGossipData) Merge(other mesh.GossipData) mesh.GossipData {
+	return d // no-op
 }
 
 func (d *ipamGossipData) Encode() [][]byte {
diff --git a/mesh/gossip.go b/mesh/gossip.go
index abfab8fa..611639cb 100644
--- a/mesh/gossip.go
+++ b/mesh/gossip.go
@@ -6,7 +6,7 @@ import (
 
 type GossipData interface {
 	Encode() [][]byte
-	Merge(GossipData)
+	Merge(GossipData) GossipData
 }
 
 type Gossip interface {
@@ -79,8 +79,7 @@ func (sender *GossipSender) Send(data GossipData) {
 	// NB: this must not be invoked concurrently
 	select {
 	case pending := <-sender.cell:
-		pending.Merge(data)
-		sender.cell <- pending
+		sender.cell <- pending.Merge(data)
 	default:
 		sender.cell <- data
 	}
diff --git a/mesh/router.go b/mesh/router.go
index 916ea5b7..c19556d7 100644
--- a/mesh/router.go
+++ b/mesh/router.go
@@ -127,17 +127,19 @@ func (router *Router) BroadcastTopologyUpdate(update []*Peer) {
 	for _, p := range update {
 		names[p.Name] = void
 	}
-
-	router.TopologyGossip.GossipBroadcast(&TopologyGossipData{
-		peers:  router.Peers,
-		update: names,
-	})
+	router.TopologyGossip.GossipBroadcast(
+		&TopologyGossipData{peers: router.Peers, update: names})
 }
 
-func (d *TopologyGossipData) Merge(other GossipData) {
-	for name := range other.(*TopologyGossipData).update {
-		d.update[name] = void
+func (d *TopologyGossipData) Merge(other GossipData) GossipData {
+	names := make(PeerNameSet)
+	for name := range d.update {
+		names[name] = void
 	}
+	for name := range other.(*TopologyGossipData).update {
+		names[name] = void
+	}
+	return &TopologyGossipData{peers: d.peers, update: names}
 }
 
 func (d *TopologyGossipData) Encode() [][]byte {
diff --git a/mesh/surrogate_gossiper.go b/mesh/surrogate_gossiper.go
index aef844a8..7204d4aa 100644
--- a/mesh/surrogate_gossiper.go
+++ b/mesh/surrogate_gossiper.go
@@ -12,8 +12,12 @@ func (d *SurrogateGossipData) Encode() [][]byte {
 	return d.messages
 }
 
-func (d *SurrogateGossipData) Merge(other GossipData) {
-	d.messages = append(d.messages, other.(*SurrogateGossipData).messages...)
+func (d *SurrogateGossipData) Merge(other GossipData) GossipData {
+	o := other.(*SurrogateGossipData)
+	messages := make([][]byte, 0, len(d.messages)+len(o.messages))
+	messages = append(messages, d.messages...)
+	messages = append(messages, o.messages...)
+	return &SurrogateGossipData{messages: messages}
 }
 
 // SurrogateGossiper ignores unicasts and relays broadcasts and gossips.
diff --git a/nameserver/entry.go b/nameserver/entry.go
index 524845a3..cd18f6de 100644
--- a/nameserver/entry.go
+++ b/nameserver/entry.go
@@ -245,12 +245,18 @@ type GossipData struct {
 	Entries
 }
 
-func (g *GossipData) Merge(o mesh.GossipData) {
+func (g *GossipData) Merge(o mesh.GossipData) mesh.GossipData {
 	other := o.(*GossipData)
-	g.Entries.merge(other.Entries)
-	if g.Timestamp < other.Timestamp {
-		g.Timestamp = other.Timestamp
+	gossip := &GossipData{
+		Entries:   make(Entries, len(g.Entries)),
+		Timestamp: g.Timestamp,
 	}
+	copy(gossip.Entries, g.Entries)
+	gossip.Entries.merge(other.Entries)
+	if gossip.Timestamp < other.Timestamp {
+		gossip.Timestamp = other.Timestamp
+	}
+	return gossip
 }
 
 func (g *GossipData) Decode(msg []byte) error {
diff --git a/nameserver/entry_test.go b/nameserver/entry_test.go
index 1e794660..57dfde02 100644
--- a/nameserver/entry_test.go
+++ b/nameserver/entry_test.go
@@ -117,7 +117,7 @@ func TestGossipDataMerge(t *testing.T) {
 	g1 := GossipData{Entries: makeEntries("AcDf")}
 	g2 := GossipData{Entries: makeEntries("BEf")}
 
-	g1.Merge(&g2)
+	g3 := g1.Merge(&g2).(*GossipData)
 
-	require.Equal(t, GossipData{Entries: makeEntries("ABcDEf")}, g1)
+	require.Equal(t, GossipData{Entries: makeEntries("ABcDEf")}, *g3)
 }