Skip to content

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

Merged
hailaz merged 1 commit intogogf:masterfrom
lingcoder:hotfix/issue2902
Jan 19, 2026
Merged

fix(util/gutil): fix false positive cycle detection in Dump (#2902)#4626
hailaz merged 1 commit intogogf:masterfrom
lingcoder:hotfix/issue2902

Conversation

@lingcoder
Copy link
Contributor

@lingcoder lingcoder commented Jan 17, 2026

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:

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:

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

  • All existing gutil tests pass (68 tests)
  • Added Test_Dump_Issue2902_SharedPointer - shared pointer not marked as cycle
  • Added Test_Dump_Issue2902_SameTypeFields - original issue scenario
  • Added benchmark tests for performance tracking
  • Verified real cycles still detected correctly

Fixes #2902

Change cycle detection from global to path-based approach.
Previously, shared references (multiple fields pointing to the same object)
were incorrectly marked as cycle dumps. Now only true cycles in the
traversal path are detected.

- Add defer delete() to remove pointer from set when function returns
- Move pointer addition inside the if block
- Add test cases for shared pointer and same type fields scenarios
@hailaz hailaz merged commit afe6beb into gogf:master Jan 19, 2026
18 checks passed
@lingcoder lingcoder deleted the hotfix/issue2902 branch February 28, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gutil.Dump has a problem with identifying circular references

2 participants