Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DeepCopy for generated API types is not safe for concurrent use #3019

Open
nrfox opened this issue Dec 6, 2023 · 1 comment
Open

DeepCopy for generated API types is not safe for concurrent use #3019

nrfox opened this issue Dec 6, 2023 · 1 comment

Comments

@nrfox
Copy link

nrfox commented Dec 6, 2023

Bug description
When calling obj.DeepCopy() on one of the generated API types concurrently, the go race detector throws some warnings about a data race.

Affected product area (please put an X in all that apply)

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[X] User Experience

Expected behavior
Expected to call obj.DeepCopy() concurrently without any data race issues. This works with k8s.io/api types.

Steps to reproduce the bug
Here's a small reproducer. Run this with the -race flag: go run -race main.go

package main

import (
	networking_v1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
	// core_v1 "k8s.io/api/core/v1"
)

func main() {
	const iterations = 10000

	done := make(chan struct{})
	obj := &networking_v1beta1.VirtualService{}
	// obj := &core_v1.Service{}
	go func() {
		for i := 0; i < iterations; i++ {
			obj.DeepCopy()
		}
		done <- struct{}{}
	}()
	go func() {
		for i := 0; i < iterations; i++ {
			obj.DeepCopy()
		}
		done <- struct{}{}
	}()
	<-done
	<-done
}

output:

==================
WARNING: DATA RACE
Read at 0x00c0001c5e48 by goroutine 7:
  istio.io/client-go/pkg/apis/networking/v1beta1.(*VirtualService).DeepCopyInto()
      /home/nrfox/go/pkg/mod/istio.io/client-go@v1.20.0/pkg/apis/networking/v1beta1/zz_generated.deepcopy.gen.go:353 +0x44
  istio.io/client-go/pkg/apis/networking/v1beta1.(*VirtualService).DeepCopy()
      /home/nrfox/go/pkg/mod/istio.io/client-go@v1.20.0/pkg/apis/networking/v1beta1/zz_generated.deepcopy.gen.go:367 +0xa4
  main.main.func1()
      /home/nrfox/Developer/redhat/kiali/testingmain/main.go:16 +0xaf

Previous write at 0x00c0001c5e48 by goroutine 8:
  sync/atomic.StoreInt64()
      /usr/local/go/src/runtime/race_amd64.s:237 +0xb
  sync/atomic.StorePointer()
      /usr/local/go/src/runtime/atomic_pointer.go:72 +0x44
  istio.io/api/networking/v1beta1.(*VirtualService).ProtoReflect()
      /home/nrfox/go/pkg/mod/istio.io/api@v1.20.0-beta.0.0.20231031143729-871b2914253f/networking/v1beta1/virtual_service.pb.go:374 +0x7c
  google.golang.org/protobuf/proto.Clone()
      /home/nrfox/go/pkg/mod/google.golang.org/protobuf@v1.31.0/proto/merge.go:53 +0x44
  istio.io/api/networking/v1beta1.(*VirtualService).DeepCopyInto()
      /home/nrfox/go/pkg/mod/istio.io/api@v1.20.0-beta.0.0.20231031143729-871b2914253f/networking/v1beta1/virtual_service_deepcopy.gen.go:10 +0x164
  istio.io/client-go/pkg/apis/networking/v1beta1.(*VirtualService).DeepCopyInto()
      /home/nrfox/go/pkg/mod/istio.io/client-go@v1.20.0/pkg/apis/networking/v1beta1/zz_generated.deepcopy.gen.go:356 +0x145
  istio.io/client-go/pkg/apis/networking/v1beta1.(*VirtualService).DeepCopy()
      /home/nrfox/go/pkg/mod/istio.io/client-go@v1.20.0/pkg/apis/networking/v1beta1/zz_generated.deepcopy.gen.go:367 +0xa4
  main.main.func2()
      /home/nrfox/Developer/redhat/kiali/testingmain/main.go:22 +0xaf

Goroutine 7 (running) created at:
  main.main()
      /home/nrfox/Developer/redhat/kiali/testingmain/main.go:14 +0x136

Goroutine 8 (running) created at:
  main.main()
      /home/nrfox/Developer/redhat/kiali/testingmain/main.go:20 +0x1de
==================
Found 1 data race(s)
exit status 66

You can comment out the VirtualService obj and uncomment the Service object to see the behavior with a k8s.io/api type. There are no data race errors.

Version (include the output of istioctl version --remote and kubectl version)
From my go.mod

	istio.io/api v1.20.0-beta.0.0.20231031143729-871b2914253f
	istio.io/client-go v1.20.0

How was Istio installed?
N/A

Environment where bug was observed (cloud vendor, OS, etc)
N/A

@howardjohn
Copy link
Member

I think removal of *out = *in fixes it, but we should understand why that is done before make any changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants