api icon indicating copy to clipboard operation
api copied to clipboard

DeepCopy for generated API types is not safe for concurrent use

Open nrfox opened this issue 2 years ago • 1 comments

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/[email protected]/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/[email protected]/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/[email protected]/networking/v1beta1/virtual_service.pb.go:374 +0x7c
  google.golang.org/protobuf/proto.Clone()
      /home/nrfox/go/pkg/mod/google.golang.org/[email protected]/proto/merge.go:53 +0x44
  istio.io/api/networking/v1beta1.(*VirtualService).DeepCopyInto()
      /home/nrfox/go/pkg/mod/istio.io/[email protected]/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/[email protected]/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/[email protected]/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

nrfox avatar Dec 06 '23 19:12 nrfox

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

howardjohn avatar Dec 06 '23 19:12 howardjohn