fix(util/gutil): fix false positive cycle detection in Dump (#2902) (#4626)

## Summary
- Fix false positive cycle detection in `gutil.Dump`
- Change from global pointer tracking to path-based cycle detection
- Shared references (multiple fields pointing to same object) no longer
incorrectly marked as cycles

## Problem
When using `gutil.Dump` with structs containing fields that share the
same `reflect.Type` (e.g., multiple `int` fields), the second field's
type was incorrectly displayed as `<cycle dump 0x...>`.

Example from issue:
```go
type User struct {
    Id   int `params:"id"`
    Name int `params:"name"`
}
fields, _ := gstructs.TagFields(&user, []string{"p", "params"})
gutil.Dump(fields)  // Second field's Type shows "<cycle dump>" instead of "int"
```

## Solution
Change cycle detection from global to path-based:
- Add `defer delete()` to remove pointer from tracking set when function
returns
- Only detect true cycles (A→B→A), not shared references (A,B both point
to C)

## Benchmark Comparison

Run benchmark with:
```bash
cd util/gutil && go test -bench=Benchmark_Dump -benchmem -run=^$
```

**Before fix (master branch):**
| Benchmark | ns/op | B/op | allocs/op |
|-----------|-------|------|-----------|
| Shallow | 4071 | 5989 | 85 |
| Nested20 | 105700 | 173993 | 1952 |
| Deep50 | 422515 | 692298 | 4869 |

**After fix (this PR):**
| Benchmark | ns/op | B/op | allocs/op |
|-----------|-------|------|-----------|
| Shallow | 4049 | 5989 | 85 |
| Nested20 | 103065 | 173990 | 1952 |
| Deep50 | 469502 | 692291 | 4869 |

**Performance impact**: 
- Memory allocation (B/op and allocs/op) is **identical**
- Execution time is within normal variance (±5-10%)
- The `defer delete()` operation is O(1), negligible compared to
reflection overhead

## Test plan
- [x] All existing `gutil` tests pass (68 tests)
- [x] Added `Test_Dump_Issue2902_SharedPointer` - shared pointer not
marked as cycle
- [x] Added `Test_Dump_Issue2902_SameTypeFields` - original issue
scenario
- [x] Added benchmark tests for performance tracking
- [x] Verified real cycles still detected correctly

Fixes #2902
This commit is contained in:
Jack Ling
2026-01-19 10:56:25 +08:00
committed by GitHub
parent 2af2342d67
commit afe6bebde7
2 changed files with 96 additions and 1 deletions

View File

@ -308,8 +308,10 @@ func doDumpStruct(in doDumpInternalInput) {
fmt.Fprintf(in.Buffer, `<cycle dump %s>`, in.PtrAddress)
return
}
// Add to set and remove when function returns (path-based cycle detection).
in.DumpedPointerSet[in.PtrAddress] = struct{}{}
defer delete(in.DumpedPointerSet, in.PtrAddress)
}
in.DumpedPointerSet[in.PtrAddress] = struct{}{}
structFields, _ := gstructs.Fields(gstructs.FieldsInput{
Pointer: in.Value,

View File

@ -13,6 +13,7 @@ import (
"github.com/gogf/gf/v2/container/gtype"
"github.com/gogf/gf/v2/frame/g"
"github.com/gogf/gf/v2/net/ghttp"
"github.com/gogf/gf/v2/os/gstructs"
"github.com/gogf/gf/v2/os/gtime"
"github.com/gogf/gf/v2/test/gtest"
"github.com/gogf/gf/v2/text/gstr"
@ -295,3 +296,95 @@ func Test_DumpJson(t *testing.T) {
gutil.DumpJson(jsonContent)
})
}
// https://github.com/gogf/gf/issues/2902
func Test_Dump_Issue2902_SharedPointer(t *testing.T) {
type Inner struct {
Value int
}
type Outer struct {
A *Inner
B *Inner
}
gtest.C(t, func(t *gtest.T) {
// Shared pointer (not a cycle) should not be marked as cycle dump.
shared := &Inner{Value: 100}
data := Outer{A: shared, B: shared}
buffer := bytes.NewBuffer(nil)
g.DumpTo(buffer, data, gutil.DumpOption{})
output := buffer.String()
// The second field should show the actual value, not "cycle dump".
// Both fields point to the same object, but it's not a cycle.
t.Assert(gstr.Contains(output, "cycle"), false)
t.Assert(gstr.Count(output, "Value"), 2)
t.Assert(gstr.Count(output, "100"), 2)
})
}
// https://github.com/gogf/gf/issues/2902
func Test_Dump_Issue2902_SameTypeFields(t *testing.T) {
type User struct {
Id int `params:"id"`
Name int `params:"name"`
}
gtest.C(t, func(t *gtest.T) {
// Fields with same type (e.g., both are int) share the same reflect.Type,
// which should not be marked as cycle dump.
var user User
fields, _ := gstructs.TagFields(&user, []string{"p", "params"})
buffer := bytes.NewBuffer(nil)
g.DumpTo(buffer, fields, gutil.DumpOption{})
output := buffer.String()
// Both fields' Type should show "int", not "cycle dump".
t.Assert(gstr.Contains(output, "cycle"), false)
t.Assert(gstr.Count(output, `Type:`), 2)
})
}
type benchStruct struct {
A int
B string
C *benchStruct
D []int
E map[string]int
}
func createBenchNested(depth int) *benchStruct {
if depth <= 0 {
return nil
}
return &benchStruct{
A: depth,
B: "test",
C: createBenchNested(depth - 1),
D: []int{1, 2, 3, 4, 5},
E: map[string]int{"x": 1, "y": 2},
}
}
var (
benchShallow = &benchStruct{A: 1, B: "test", D: []int{1, 2, 3}, E: map[string]int{"a": 1}}
benchNested20 = createBenchNested(20)
benchDeep50 = createBenchNested(50)
)
func Benchmark_Dump_Shallow(b *testing.B) {
for i := 0; i < b.N; i++ {
var buf bytes.Buffer
gutil.DumpTo(&buf, benchShallow, gutil.DumpOption{})
}
}
func Benchmark_Dump_Nested20(b *testing.B) {
for i := 0; i < b.N; i++ {
var buf bytes.Buffer
gutil.DumpTo(&buf, benchNested20, gutil.DumpOption{})
}
}
func Benchmark_Dump_Deep50(b *testing.B) {
for i := 0; i < b.N; i++ {
var buf bytes.Buffer
gutil.DumpTo(&buf, benchDeep50, gutil.DumpOption{})
}
}