mock: fix data race in Arguments.Diff for pointer-like arguments

Arguments.Diff uses fmt.Sprintf("%v") to format arguments for diff output.
When arguments contain pointers, maps, slices, or other reference types that
are concurrently modified by other goroutines, this causes data races:

- For maps: a fatal "concurrent map iteration and map write" error that
  crashes the entire test process (non-recoverable)
- For pointers/slices: data races detectable by -race, and potential panics

This commit introduces safeFormatArg() which handles each case:
- Maps: uses %p (address-only) since map iteration cannot be made safe
  without external synchronization
- Pointers/slices: attempts %v with panic recovery, falls back to %p
- All other types: unchanged %v behavior (value types, no race possible)

This preserves the full %v diff output for the common case (non-concurrent
arguments, value types, pointers/slices without races) while preventing
testify itself from being the source of data races in user tests.

Fixes stretchr#1597
Related: stretchr#1598 (alternative approach using %p for all pointers)

Signed-off-by: Pavel Zeman <pavel.zeman@mattermost.com>

Co-authored-by: Claude <claude@anthropic.com>

Author: pavelzeman

mock/mock.go

@@ -977,15 +977,15 @@ func (args Arguments) Diff(objects []interface{}) (string, int) {
 			actualFmt = "(Missing)"
 		} else {
 			actual = objects[i]
-			actualFmt = fmt.Sprintf("(%[1]T=%[1]v)", actual)
+			actualFmt = safeFormatArg(actual)
 		}
 
 		if len(args) <= i {
 			expected = "(Missing)"
 			expectedFmt = "(Missing)"
 		} else {
 			expected = args[i]
-			expectedFmt = fmt.Sprintf("(%[1]T=%[1]v)", expected)
+			expectedFmt = safeFormatArg(expected)
 		}
 
 		if matcher, ok := expected.(argumentMatcher); ok {
@@ -1310,3 +1310,44 @@ func isFuncSame(f1, f2 *runtime.Func) bool {
 
 	return f1File == f2File && f1Loc == f2Loc
 }
+
+// safeFormatArg formats an argument for display in diff output, handling
+// concurrent modification safely.
+//
+// Maps require special handling because concurrent read+write triggers a fatal
+// runtime error ("concurrent map iteration and map write") that cannot be
+// recovered. For maps, we use %p to print only the address.
+//
+// Pointers and slices can also race with concurrent writers, but those produce
+// recoverable panics. We attempt %v formatting with panic recovery for those.
+//
+// All other types are value types and safe to format directly.
+//
+// See https://github.com/stretchr/testify/issues/1597
+func safeFormatArg(v interface{}) string {
+	if v == nil {
+		return fmt.Sprintf("(%[1]T=%[1]v)", v)
+	}
+	kind := reflect.TypeOf(v).Kind()
+	switch kind {
+	case reflect.Map:
+		// Maps trigger a non-recoverable fatal error on concurrent read+write.
+		// Use address-only format to avoid iterating the map.
+		return fmt.Sprintf("(%T=%p)", v, v)
+	case reflect.Ptr, reflect.Slice:
+		// Pointers and slices can panic on concurrent modification, but
+		// the panic is recoverable. Attempt full format with fallback.
+		var result string
+		func() {
+			defer func() {
+				if r := recover(); r != nil {
+					result = fmt.Sprintf("(%T=%p)", v, v)
+				}
+			}()
+			result = fmt.Sprintf("(%[1]T=%[1]v)", v)
+		}()
+		return result
+	default:
+		return fmt.Sprintf("(%[1]T=%[1]v)", v)
+	}
+}

mock/mock_test.go

@@ -2044,6 +2044,81 @@ func Test_Arguments_Diff_WithArgMatcher(t *testing.T) {
 	assert.Contains(t, diff, `No differences.`)
 }
 
+// Test_Arguments_Diff_ConcurrentPointerModification verifies that
+// Arguments.Diff does not race when a pointer argument is concurrently
+// modified. This is a regression test for https://github.com/stretchr/testify/issues/1597.
+// Adapted from https://github.com/stretchr/testify/pull/1598.
+func Test_Arguments_Diff_ConcurrentPointerModification(t *testing.T) {
+	t.Parallel()
+
+	type data struct {
+		Value string
+	}
+
+	arg := &data{Value: "original"}
+	args := Arguments([]interface{}{Anything})
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		for i := 0; i < 1000; i++ {
+			arg.Value = fmt.Sprintf("modified-%d", i)
+		}
+	}()
+
+	// Without the fix, this races with the goroutine above because
+	// fmt.Sprintf("%v") traverses *data while the goroutine writes to it.
+	for i := 0; i < 100; i++ {
+		args.Diff([]interface{}{arg})
+	}
+	<-done
+}
+
+// Test_Arguments_Diff_ConcurrentMapModification verifies that Arguments.Diff
+// does not race when a map argument is concurrently modified.
+// Raised by @brackendawson in https://github.com/stretchr/testify/pull/1598#discussion_r1869482623.
+func Test_Arguments_Diff_ConcurrentMapModification(t *testing.T) {
+	t.Parallel()
+
+	arg := map[string]string{"key": "original"}
+	args := Arguments([]interface{}{Anything})
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		for i := 0; i < 1000; i++ {
+			arg["key"] = fmt.Sprintf("modified-%d", i)
+		}
+	}()
+
+	for i := 0; i < 100; i++ {
+		args.Diff([]interface{}{arg})
+	}
+	<-done
+}
+
+// Test_Arguments_Diff_ConcurrentSliceModification verifies that Arguments.Diff
+// does not race when a slice argument is concurrently modified.
+func Test_Arguments_Diff_ConcurrentSliceModification(t *testing.T) {
+	t.Parallel()
+
+	arg := []string{"original"}
+	args := Arguments([]interface{}{Anything})
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		for i := 0; i < 1000; i++ {
+			arg[0] = fmt.Sprintf("modified-%d", i)
+		}
+	}()
+
+	for i := 0; i < 100; i++ {
+		args.Diff([]interface{}{arg})
+	}
+	<-done
+}
+
 func Test_Arguments_Assert(t *testing.T) {
 	t.Parallel()