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

fix: lock tenant map to avoid race #702

Merged
merged 1 commit into from
Jun 29, 2023
Merged

fix: lock tenant map to avoid race #702

merged 1 commit into from
Jun 29, 2023

Conversation

cbuto
Copy link
Contributor

@cbuto cbuto commented Jun 29, 2023

Fixes another race saw here:

https://github.com/helm/chartmuseum/actions/runs/5413041445/jobs/9838017827#step:5:6653

WARNING: DATA RACE
Write at 0x00c00099d650 by goroutine 254:
  runtime.mapassign_faststr()
      /opt/hostedtoolcache/go/1.20.5/x64/src/runtime/map_faststr.go:203 +0x0
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).initCacheEntry()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:345 +0x2fc
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).getIndexFile()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/index.go:34 +0xda
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).getIndexFileRequestHandler()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/handlers.go:123 +0x1cb
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).getIndexFileRequestHandler-fm()
      <autogenerated>:1 +0x44
  helm.sh/chartmuseum/pkg/chartmuseum/router.(*Router).rootHandler()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/router/router.go:250 +0x8c2
  helm.sh/chartmuseum/pkg/chartmuseum/router.(*Router).rootHandler-fm()
      <autogenerated>:1 +0x44
  github.com/gin-gonic/gin.(*Context).Next()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/context.go:174 +0x207
  github.com/gin-contrib/size.RequestSizeLimiter.func1()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-contrib/size/size.go:88 +0x19b
  github.com/gin-gonic/gin.(*Context).Next()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/context.go:174 +0x238
  helm.sh/chartmuseum/pkg/chartmuseum/router.requestWrapper.func1()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/router/middleware.go:48 +0x1aa
  github.com/gin-gonic/gin.(*Context).Next()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/context.go:174 +0x151
  github.com/gin-gonic/gin.CustomRecoveryWithWriter.func1()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/recovery.go:102 +0xbd
  github.com/gin-gonic/gin.(*Context).Next()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/context.go:174 +0x109
  github.com/gin-gonic/gin.serveError()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/gin.go:656 +0x8d
  github.com/gin-gonic/gin.(*Engine).handleHTTPRequest()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/gin.go:649 +0x8fd
  github.com/gin-gonic/gin.(*Engine).HandleContext()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/gin-gonic/gin/gin.go:587 +0x3b0
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServerTestSuite).doRequest()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:102 +0x62b
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServerTestSuite).testAllRoutes()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:1077 +0x36a
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServerTestSuite).TestRoutes()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/server_test.go:1050 +0x331
  runtime.call16()
      /opt/hostedtoolcache/go/1.20.5/x64/src/runtime/asm_amd64.s:728 +0x48
  reflect.Value.Call()
      /opt/hostedtoolcache/go/1.20.5/x64/src/reflect/value.go:370 +0xc7
  github.com/stretchr/testify/suite.Run.func1()
      /home/runner/work/chartmuseum/chartmuseum/vendor/github.com/stretchr/testify/suite/suite.go:197 +0x70d
  testing.tRunner()
      /opt/hostedtoolcache/go/1.20.5/x64/src/testing/testing.go:1576 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.20.5/x64/src/testing/testing.go:1629 +0x47

Previous read at 0x00c00099d650 by goroutine 335:
  runtime.mapaccess1_faststr()
      /opt/hostedtoolcache/go/1.20.5/x64/src/runtime/map_faststr.go:13 +0x0
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).getChartList()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:103 +0xf2
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).refreshCacheEntry()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:595 +0xcb
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).rebuildIndexForTenant()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:591 +0x397
  helm.sh/chartmuseum/pkg/chartmuseum/server/multitenant.(*MultiTenantServer).rebuildIndex.func2()
      /home/runner/work/chartmuseum/chartmuseum/pkg/chartmuseum/server/multitenant/cache.go:576 +0x58

@cbuto cbuto marked this pull request as ready for review June 29, 2023 17:11
@cbuto cbuto requested review from jdolitsky and scbizu June 29, 2023 17:11
@jdolitsky jdolitsky merged commit d459f51 into helm:main Jun 29, 2023
2 checks passed
@kycheng
Copy link

kycheng commented Dec 8, 2023

@cbuto I also encountered the same problem, will this change be fixed in 0.14.1?

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

Successfully merging this pull request may close these issues.

3 participants