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

trace: fix race condition #279

Merged

Commits on Apr 6, 2023

  1. trace: fix race condition

    Found with `go test -race -run=TestPatchConflicts ./test/integration/apiserver`
    in Kubernetes.
    
    writeTraceSteps gets called while the mutex of t is locked, so iterating over
    t.traceItems is fine. But then it calls stepOrTrace.time = Trace.time of
    another instance. Retrieving that time is done without locking the mutex of
    that other instance and thus races with setting the end time of that trace in
    End.
    
    WARNING: DATA RACE
    Write at 0x00c009fed898 by goroutine 124291:
      k8s.io/kubernetes/vendor/k8s.io/utils/trace.(*Trace).Log()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/utils/trace/trace.go:175 +0xc4
      k8s.io/kubernetes/vendor/k8s.io/utils/trace.(*Trace).LogIfLong()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/utils/trace/trace.go:194 +0xc4
      k8s.io/kubernetes/vendor/k8s.io/component-base/tracing.(*Span).End()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/component-base/tracing/tracing.go:67 +0x8f
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/etcd3.(*store).GuaranteedUpdate.func2()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/etcd3/store.go:384 +0x3e
      runtime.deferreturn()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/.gimme/versions/go1.20.2.linux.amd64/src/runtime/panic.go:476 +0x32
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/cacher.(*Cacher).GuaranteedUpdate()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/storage/cacher/cacher.go:839 +0x1cc
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/registry/generic/registry.(*DryRunnableStorage).GuaranteedUpdate()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go:93 +0x321
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/registry/generic/registry.(*Store).Update()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/registry/generic/registry/store.go:559 +0x858
      k8s.io/kubernetes/pkg/registry/core/secret/storage.(*REST).Update()
          <autogenerated>:1 +0xfe
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers.(*patcher).patchResource.func2()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers/patch.go:666 +0x1f1
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers.(*patcher).patchResource.func3()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers/patch.go:672 +0x57
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers/finisher.finishRequest.func1()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers/finisher/finisher.go:117 +0xfa
    
    Previous read at 0x00c009fed898 by goroutine 124286:
      k8s.io/kubernetes/vendor/k8s.io/utils/trace.(*Trace).time()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/utils/trace/trace.go:110 +0x3e
      k8s.io/kubernetes/vendor/k8s.io/utils/trace.(*Trace).writeTraceSteps()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/utils/trace/trace.go:235 +0x17a
      k8s.io/kubernetes/vendor/k8s.io/utils/trace.(*Trace).logTrace()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/utils/trace/trace.go:216 +0x4c7
      k8s.io/kubernetes/vendor/k8s.io/utils/trace.(*Trace).Log()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/utils/trace/trace.go:179 +0x118
      k8s.io/kubernetes/vendor/k8s.io/utils/trace.(*Trace).LogIfLong()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/utils/trace/trace.go:194 +0xc4
      k8s.io/kubernetes/vendor/k8s.io/component-base/tracing.(*Span).End()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/component-base/tracing/tracing.go:67 +0x8f
      k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers.PatchResource.func1.1()
          /home/prow/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/endpoints/handlers/patch.go:69 +0x3e
      ...
    pohly committed Apr 6, 2023
    Configuration menu
    Copy the full SHA
    43193cd View commit details
    Browse the repository at this point in the history