lib/discover: Add instance ID to local discovery (fixes #3278)

A random "instance ID" is generated on each start of the local discovery
service. The instance ID is included in the announcement. When we see a
new instance ID we treat is a new device and respond with an
announcement of our own. Hence devices get to know each other quickly on
restart.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3385

lib/discover/discover.go

@@ -26,6 +26,7 @@ type CacheEntry struct {
 	when       time.Time // When did we get the result
 	found      bool      // Is it a success (cacheTime applies) or a failure (negCacheTime applies)?
 	validUntil time.Time // Validity time, overrides normal calculation
+	instanceID int64     // for local discovery, the instance ID (random on each restart)
 }
 
 // A FinderService is a Finder that has background activity and must be run as

lib/discover/local.go

@@ -22,6 +22,7 @@ import (
 	"github.com/syncthing/syncthing/lib/beacon"
 	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/protocol"
+	"github.com/syncthing/syncthing/lib/rand"
 	"github.com/thejerf/suture"
 )
 
@@ -114,8 +115,9 @@ func (c *localClient) Error() error {
 
 func (c *localClient) announcementPkt() Announce {
 	return Announce{
-		ID:        c.myID[:],
+		ID:         c.myID[:],
-		Addresses: c.addrList.AllAddresses(),
+		Addresses:  c.addrList.AllAddresses(),
+		InstanceID: rand.Int63(),
 	}
 }
 
@@ -194,9 +196,11 @@ func (c *localClient) registerDevice(src net.Addr, device Announce) bool {
 	copy(id[:], device.ID)
 
 	// Remember whether we already had a valid cache entry for this device.
+	// If the instance ID has changed the remote device has restarted since
+	// we last heard from it, so we should treat it as a new device.
 
 	ce, existsAlready := c.Get(id)
-	isNewDevice := !existsAlready || time.Since(ce.when) > CacheLifeTime
+	isNewDevice := !existsAlready || time.Since(ce.when) > CacheLifeTime || ce.instanceID != device.InstanceID
 
 	// Any empty or unspecified addresses should be set to the source address
 	// of the announcement. We also skip any addresses we can't parse.
@@ -245,9 +249,10 @@ func (c *localClient) registerDevice(src net.Addr, device Announce) bool {
 	}
 
 	c.Set(id, CacheEntry{
-		Addresses: validAddresses,
+		Addresses:  validAddresses,
-		when:      time.Now(),
+		when:       time.Now(),
-		found:     true,
+		found:      true,
+		instanceID: device.InstanceID,
 	})
 
 	if isNewDevice {

lib/discover/local.pb.go

@@ -30,8 +30,9 @@ var _ = math.Inf
 const _ = proto.GoGoProtoPackageIsVersion1
 
 type Announce struct {
-	ID        []byte   `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"`
+	ID         []byte   `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"`
-	Addresses []string `protobuf:"bytes,2,rep,name=addresses" json:"addresses,omitempty"`
+	Addresses  []string `protobuf:"bytes,2,rep,name=addresses" json:"addresses,omitempty"`
+	InstanceID int64    `protobuf:"varint,3,opt,name=instance_id,json=instanceId,proto3" json:"instance_id,omitempty"`
 }
 
 func (m *Announce) Reset()                    { *m = Announce{} }
@@ -78,6 +79,11 @@ func (m *Announce) MarshalTo(data []byte) (int, error) {
 			i += copy(data[i:], s)
 		}
 	}
+	if m.InstanceID != 0 {
+		data[i] = 0x18
+		i++
+		i = encodeVarintLocal(data, i, uint64(m.InstanceID))
+	}
 	return i, nil
 }
 
@@ -121,6 +127,9 @@ func (m *Announce) ProtoSize() (n int) {
 			n += 1 + l + sovLocal(uint64(l))
 		}
 	}
+	if m.InstanceID != 0 {
+		n += 1 + sovLocal(uint64(m.InstanceID))
+	}
 	return n
 }
 
@@ -226,6 +235,25 @@ func (m *Announce) Unmarshal(data []byte) error {
 			}
 			m.Addresses = append(m.Addresses, string(data[iNdEx:postIndex]))
 			iNdEx = postIndex
+		case 3:
+			if wireType != 0 {
+				return fmt.Errorf("proto: wrong wireType = %d for field InstanceID", wireType)
+			}
+			m.InstanceID = 0
+			for shift := uint(0); ; shift += 7 {
+				if shift >= 64 {
+					return ErrIntOverflowLocal
+				}
+				if iNdEx >= l {
+					return io.ErrUnexpectedEOF
+				}
+				b := data[iNdEx]
+				iNdEx++
+				m.InstanceID |= (int64(b) & 0x7F) << shift
+				if b < 0x80 {
+					break
+				}
+			}
 		default:
 			iNdEx = preIndex
 			skippy, err := skipLocal(data[iNdEx:])
@@ -353,16 +381,18 @@ var (
 )
 
 var fileDescriptorLocal = []byte{
-	// 161 bytes of a gzipped FileDescriptorProto
+	// 194 bytes of a gzipped FileDescriptorProto
 	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0xe2, 0xe2, 0xce, 0xc9, 0x4f, 0x4e,
 	0xcc, 0xd1, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0xe2, 0x48, 0xc9, 0x2c, 0x4e, 0xce, 0x2f, 0x4b,
 	0x2d, 0x92, 0xd2, 0x4d, 0xcf, 0x2c, 0xc9, 0x28, 0x4d, 0xd2, 0x4b, 0xce, 0xcf, 0xd5, 0x4f, 0xcf,
 	0x4f, 0xcf, 0xd7, 0x07, 0x2b, 0x48, 0x2a, 0x4d, 0x03, 0xf3, 0xc0, 0x1c, 0x30, 0x0b, 0xa2, 0x51,
-	0xc9, 0x81, 0x8b, 0xc3, 0x31, 0x2f, 0x2f, 0xbf, 0x34, 0x2f, 0x39, 0x55, 0x48, 0x8c, 0x8b, 0x29,
+	0xa9, 0x90, 0x8b, 0xc3, 0x31, 0x2f, 0x2f, 0xbf, 0x34, 0x2f, 0x39, 0x55, 0x48, 0x8c, 0x8b, 0x29,
 	0x33, 0x45, 0x82, 0x51, 0x81, 0x51, 0x83, 0xc7, 0x89, 0xed, 0xd1, 0x3d, 0x79, 0x26, 0x4f, 0x97,
 	0x20, 0xa0, 0x88, 0x90, 0x0c, 0x17, 0x67, 0x62, 0x4a, 0x4a, 0x51, 0x6a, 0x71, 0x71, 0x6a, 0xb1,
-	0x04, 0x93, 0x02, 0xb3, 0x06, 0x67, 0x10, 0x42, 0xc0, 0x49, 0xe4, 0xc4, 0x43, 0x39, 0x86, 0x13,
+	0x04, 0x93, 0x02, 0xb3, 0x06, 0x67, 0x10, 0x42, 0x40, 0x48, 0x9f, 0x8b, 0x3b, 0x33, 0xaf, 0xb8,
-	0x8f, 0xe4, 0x18, 0x2f, 0x00, 0xf1, 0x83, 0x47, 0x72, 0x0c, 0x0b, 0x1e, 0xcb, 0x31, 0x26, 0xb1,
+	0x24, 0x11, 0x68, 0x42, 0x3c, 0x50, 0x3b, 0x33, 0x50, 0x3b, 0xb3, 0x13, 0x1f, 0x50, 0x3b, 0x97,
-	0x81, 0x8d, 0x37, 0x06, 0x04, 0x00, 0x00, 0xff, 0xff, 0xc9, 0xec, 0xea, 0xbc, 0xa6, 0x00, 0x00,
+	0x27, 0x54, 0x18, 0x68, 0x0c, 0x17, 0x4c, 0x89, 0x67, 0x8a, 0x93, 0xc8, 0x89, 0x87, 0x72, 0x0c,
-	0x00,
+	0x27, 0x1e, 0xc9, 0x31, 0x5e, 0x00, 0xe2, 0x07, 0x8f, 0xe4, 0x18, 0x16, 0x3c, 0x96, 0x63, 0x4c,
+	0x62, 0x03, 0xbb, 0xc7, 0x18, 0x10, 0x00, 0x00, 0xff, 0xff, 0x91, 0x3f, 0x96, 0x25, 0xd7, 0x00,
+	0x00, 0x00,
 }

lib/discover/local.proto

@@ -9,6 +9,7 @@ option (gogoproto.sizer_all) = false;
 option (gogoproto.protosizer_all) = true;
 
 message Announce {
-    bytes           id        = 1 [(gogoproto.customname) = "ID"];
+    bytes           id          = 1 [(gogoproto.customname) = "ID"];
-    repeated string addresses = 2;
+    repeated string addresses   = 2;
+    int64           instance_id = 3 [(gogoproto.customname) = "InstanceID"];
 }

lib/discover/local_test.go

@@ -0,0 +1,75 @@
+package discover
+
+import (
+	"net"
+	"testing"
+
+	"github.com/syncthing/syncthing/lib/protocol"
+)
+
+func TestRandomLocalInstanceID(t *testing.T) {
+	c, err := NewLocal(protocol.LocalDeviceID, ":0", &fakeAddressLister{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	go c.Serve()
+	defer c.Stop()
+
+	lc := c.(*localClient)
+
+	p0 := lc.announcementPkt()
+	p1 := lc.announcementPkt()
+	if p0.InstanceID == p1.InstanceID {
+		t.Error("each generated packet should have a new instance id")
+	}
+}
+
+func TestLocalInstanceIDShouldTriggerNew(t *testing.T) {
+	c, err := NewLocal(protocol.LocalDeviceID, ":0", &fakeAddressLister{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	lc := c.(*localClient)
+	src := &net.UDPAddr{IP: []byte{10, 20, 30, 40}, Port: 50}
+
+	new := lc.registerDevice(src, Announce{
+		ID:         []byte{10, 20, 30, 40, 50, 60, 70, 80, 90},
+		Addresses:  []string{"tcp://0.0.0.0:22000"},
+		InstanceID: 1234567890,
+	})
+
+	if !new {
+		t.Fatal("first register should be new")
+	}
+
+	new = lc.registerDevice(src, Announce{
+		ID:         []byte{10, 20, 30, 40, 50, 60, 70, 80, 90},
+		Addresses:  []string{"tcp://0.0.0.0:22000"},
+		InstanceID: 1234567890,
+	})
+
+	if new {
+		t.Fatal("second register should not be new")
+	}
+
+	new = lc.registerDevice(src, Announce{
+		ID:         []byte{42, 10, 20, 30, 40, 50, 60, 70, 80, 90},
+		Addresses:  []string{"tcp://0.0.0.0:22000"},
+		InstanceID: 1234567890,
+	})
+
+	if !new {
+		t.Fatal("new device ID should be new")
+	}
+
+	new = lc.registerDevice(src, Announce{
+		ID:         []byte{10, 20, 30, 40, 50, 60, 70, 80, 90},
+		Addresses:  []string{"tcp://0.0.0.0:22000"},
+		InstanceID: 91234567890,
+	})
+
+	if !new {
+		t.Fatal("new instance ID should be new")
+	}
+}