-
Notifications
You must be signed in to change notification settings - Fork 706
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
The are multiple Copy() methods in the code base which are used to create deep copies of a struct. Any bugs in them can manifest as data races in concurrent code. Add a quicktest checker which ensures that two variables are a deep copy of each other: values match, but all locations in memory differ. Fix a variety of problems with the existing Copy() implementations. They all allow copying a nil struct and copy all elements. There are exceptions to the deep copy rule: - MapSpec.Contents is not deep copied because we can't easily make copies of interface values. This is documented already. - MapSpec.Extra is an immutable bytes.Reader and therefore only needs a shallow copy. - ProgramSpec.AttachTarget is a Program, which is (currently) safe for concurrent use. Fixes #1517 Signed-off-by: Lorenz Bauer <[email protected]>
- Loading branch information
Showing
9 changed files
with
353 additions
and
30 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| package testutils | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "fmt" | ||
| "reflect" | ||
|
|
||
| "github.com/go-quicktest/qt" | ||
| ) | ||
|
|
||
| // IsDeepCopy checks that got is a deep copy of want. | ||
| // | ||
| // All primitive values must be equal, but pointers must be distinct. | ||
| // This is different from [reflect.DeepEqual] which will accept equal pointer values. | ||
| // That is, reflect.DeepEqual(a, a) is true, while IsDeepCopy(a, a) is false. | ||
| func IsDeepCopy[T any](got, want T) qt.Checker { | ||
| return &deepCopyChecker[T]{got, want, make(map[pair]struct{})} | ||
| } | ||
|
|
||
| type pair struct { | ||
| got, want reflect.Value | ||
| } | ||
|
|
||
| type deepCopyChecker[T any] struct { | ||
| got, want T | ||
| visited map[pair]struct{} | ||
| } | ||
|
|
||
| func (dcc *deepCopyChecker[T]) Check(_ func(key string, value any)) error { | ||
| return dcc.check(reflect.ValueOf(dcc.got), reflect.ValueOf(dcc.want)) | ||
| } | ||
|
|
||
| func (dcc *deepCopyChecker[T]) check(got, want reflect.Value) error { | ||
| switch want.Kind() { | ||
| case reflect.Interface: | ||
| return dcc.check(got.Elem(), want.Elem()) | ||
|
|
||
| case reflect.Pointer: | ||
| if got.IsNil() && want.IsNil() { | ||
| return nil | ||
| } | ||
|
|
||
| if got.IsNil() { | ||
| return fmt.Errorf("expected non-nil pointer") | ||
| } | ||
|
|
||
| if want.IsNil() { | ||
| return fmt.Errorf("expected nil pointer") | ||
| } | ||
|
|
||
| if got.UnsafePointer() == want.UnsafePointer() { | ||
| return fmt.Errorf("equal pointer values") | ||
| } | ||
|
|
||
| switch want.Type() { | ||
| case reflect.TypeOf((*bytes.Reader)(nil)): | ||
| // bytes.Reader doesn't allow modifying it's contents, so we | ||
| // allow a shallow copy. | ||
| return nil | ||
| } | ||
|
|
||
| if _, ok := dcc.visited[pair{got, want}]; ok { | ||
| // Deal with recursive types. | ||
| return nil | ||
| } | ||
|
|
||
| dcc.visited[pair{got, want}] = struct{}{} | ||
| return dcc.check(got.Elem(), want.Elem()) | ||
|
|
||
| case reflect.Slice: | ||
| if got.IsNil() && want.IsNil() { | ||
| return nil | ||
| } | ||
|
|
||
| if got.IsNil() { | ||
| return fmt.Errorf("expected non-nil slice") | ||
| } | ||
|
|
||
| if want.IsNil() { | ||
| return fmt.Errorf("expected nil slice") | ||
| } | ||
|
|
||
| if got.Len() != want.Len() { | ||
| return fmt.Errorf("expected %d elements, got %d", want.Len(), got.Len()) | ||
| } | ||
|
|
||
| if want.Len() == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| if got.UnsafePointer() == want.UnsafePointer() { | ||
| return fmt.Errorf("equal backing memory") | ||
| } | ||
|
|
||
| fallthrough | ||
|
|
||
| case reflect.Array: | ||
| for i := 0; i < want.Len(); i++ { | ||
| if err := dcc.check(got.Index(i), want.Index(i)); err != nil { | ||
| return fmt.Errorf("index %d: %w", i, err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
|
|
||
| case reflect.Struct: | ||
| for i := 0; i < want.NumField(); i++ { | ||
| if err := dcc.check(got.Field(i), want.Field(i)); err != nil { | ||
| return fmt.Errorf("%q: %w", want.Type().Field(i).Name, err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
|
|
||
| case reflect.Map: | ||
| if got.Len() != want.Len() { | ||
| return fmt.Errorf("expected %d items, got %d", want.Len(), got.Len()) | ||
| } | ||
|
|
||
| if got.UnsafePointer() == want.UnsafePointer() { | ||
| return fmt.Errorf("maps are equal") | ||
| } | ||
|
|
||
| iter := want.MapRange() | ||
| for iter.Next() { | ||
| key := iter.Key() | ||
| got := got.MapIndex(iter.Key()) | ||
| if !got.IsValid() { | ||
| return fmt.Errorf("key %v is missing", key) | ||
| } | ||
|
|
||
| want := iter.Value() | ||
| if err := dcc.check(got, want); err != nil { | ||
| return fmt.Errorf("key %v: %w", key, err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
|
|
||
| case reflect.Chan, reflect.UnsafePointer: | ||
| return fmt.Errorf("%s is not supported", want.Type()) | ||
|
|
||
| default: | ||
| // Compare by value as usual. | ||
| if !got.Equal(want) { | ||
| return fmt.Errorf("%#v is not equal to %#v", got, want) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
| } | ||
|
|
||
| func (dcc *deepCopyChecker[T]) Args() []qt.Arg { | ||
| return []qt.Arg{ | ||
| {Name: "got", Value: dcc.got}, | ||
| {Name: "want", Value: dcc.want}, | ||
| } | ||
| } |
Oops, something went wrong.