Skip to content

fix: lock tenant map to avoid race#702

Merged
jdolitsky merged 1 commit into
helm:mainfrom
cbuto:fix-race
Jun 29, 2023
Merged

fix: lock tenant map to avoid race#702
jdolitsky merged 1 commit into
helm:mainfrom
cbuto:fix-race

Conversation

@cbuto

@cbuto cbuto commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

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
@kycheng

kycheng commented Dec 8, 2023

Copy link
Copy Markdown

@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