util/deephash: prevent infinite loop on map cycle

Fixes #2340

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

util/deephash/deephash.go

@@ -169,6 +169,28 @@ func (h *hasher) print(v reflect.Value) (acyclic bool) {
 	case reflect.Interface:
 		return h.print(v.Elem())
 	case reflect.Map:
+		// TODO(bradfitz): ideally we'd avoid these map
+		// operations to detect cycles if we knew from the map
+		// element type that there no way to form a cycle,
+		// which is the common case. Notably, we don't care
+		// about hashing the same map+contents twice in
+		// different parts of the tree. In fact, we should
+		// ideally. (And this prevents it) We should only stop
+		// hashing when there's a cycle.  What we should
+		// probably do is make sure we enumerate the data
+		// structure tree is a fixed order and then give each
+		// pointer an increasing number, and when we hit a
+		// dup, rather than emitting nothing, we should emit a
+		// "value #12" reference. Which implies that all things
+		// emit to the bufio.Writer should be type-tagged so
+		// we can distinguish loop references without risk of
+		// collisions.
+		ptr := v.Pointer()
+		if visited[ptr] {
+			return false
+		}
+		visited[ptr] = true
+
 		if h.hashMapAcyclic(v) {
 			return true
 		}

util/deephash/deephash_test.go

@@ -226,3 +226,15 @@ func TestSHA256EqualHex(t *testing.T) {
 		}
 	}
 }
+
+// verify this doesn't loop forever, as it used to (Issue 2340)
+func TestMapCyclicFallback(t *testing.T) {
+	type T struct {
+		M map[string]interface{}
+	}
+	v := &T{
+		M: map[string]interface{}{},
+	}
+	v.M["m"] = v.M
+	Hash(v)
+}