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

ctlstore regularly produces races detectable by the Go race detector #125

Open
extemporalgenome opened this issue Sep 20, 2023 · 0 comments

Comments

@extemporalgenome
Copy link

WARNING: DATA RACE
--
  | Write at 0x000000e93ac0 by goroutine 22:
  | database/sql.convertAssignRows()
  | /go/src/database/sql/convert.go:381 +0x1aa7
  | database/sql.(*Rows).Scan()
  | /go/src/database/sql/sql.go:3326 +0x6e4
  | github.com/segmentio/ctlstore/pkg/scanfunc.New.scanFuncStruct.func1()
  | /var/lib/buildkite-agent/go/pkg/mod/github.com/segmentio/[email protected]/pkg/scanfunc/scan_func.go:88 +0xa7
  | github.com/segmentio/ctlstore.(*Rows).Scan()
  | /var/lib/buildkite-agent/go/pkg/mod/github.com/segmentio/[email protected]/rows.go:58 +0xb9
  | github.com/segmentio/profile-data-services/internal/traitcatalog.(*Client).scanTraitCatalog()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog.go:88 +0x27c
  | github.com/segmentio/profile-data-services/internal/traitcatalog.(*Client).TraitspaceDefinitions()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog.go:76 +0x1b1
  | github.com/segmentio/profile-data-services/internal/traitcatalog.TestTraitCatalog.func7()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog_test.go:187 +0x351
  | testing.tRunner()
  | /go/src/testing/testing.go:1595 +0x238
  | testing.(*T).Run.func1()
  | /go/src/testing/testing.go:1648 +0x44
  |  
  | Previous write at 0x000000e93ac0 by goroutine 19:
  | database/sql.convertAssignRows()
  | /go/src/database/sql/convert.go:381 +0x1aa7
  | database/sql.(*Rows).Scan()
  | /go/src/database/sql/sql.go:3326 +0x6e4
  | github.com/segmentio/ctlstore/pkg/scanfunc.New.scanFuncStruct.func1()
  | /var/lib/buildkite-agent/go/pkg/mod/github.com/segmentio/[email protected]/pkg/scanfunc/scan_func.go:88 +0xa7
  | github.com/segmentio/ctlstore.(*Rows).Scan()
  | /var/lib/buildkite-agent/go/pkg/mod/github.com/segmentio/[email protected]/rows.go:58 +0xb9
  | github.com/segmentio/profile-data-services/internal/traitcatalog.(*Client).scanTraitCatalog()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog.go:88 +0x27c
  | github.com/segmentio/profile-data-services/internal/traitcatalog.(*Client).Definitions()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog.go:67 +0x165
  | github.com/segmentio/profile-data-services/internal/traitcatalog.TestTraitCatalog.func4()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog_test.go:113 +0x344
  | testing.tRunner()
  | /go/src/testing/testing.go:1595 +0x238
  | testing.(*T).Run.func1()
  | /go/src/testing/testing.go:1648 +0x44
  |  
  | Goroutine 22 (running) created at:
  | testing.(*T).Run()
  | /go/src/testing/testing.go:1648 +0x82a
  | github.com/segmentio/profile-data-services/internal/traitcatalog.TestTraitCatalog()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog_test.go:163 +0x516
  | testing.tRunner()
  | /go/src/testing/testing.go:1595 +0x238
  | testing.(*T).Run.func1()
  | /go/src/testing/testing.go:1648 +0x44
  |  
  | Goroutine 19 (running) created at:
  | testing.(*T).Run()
  | /go/src/testing/testing.go:1648 +0x82a
  | github.com/segmentio/profile-data-services/internal/traitcatalog.TestTraitCatalog()
  | /var/lib/buildkite-agent/builds/buildkite-ci-0f362bc367e6f2b76/segment/profile-data-services/internal/traitcatalog/traitcatalog_test.go:89 +0x356
  | testing.tRunner()
  | /go/src/testing/testing.go:1595 +0x238
  | testing.(*T).Run.func1()
  | /go/src/testing/testing.go:1648 +0x44
  | ==================
  | FAIL: TestTraitCatalog (0.01s) | 0s
  | --- FAIL: TestTraitCatalog/GetTraitDefinitionForNamespace_found (0.00s)
  | testing.go:1465: race detected during execution of test
  | --- FAIL: TestTraitCatalog/GetTraitDefinitionForNamespaceInTraitSpace_found (0.00s)
  | testing.go:1465: race detected during execution of test

This seems to be due to avoidable cleverness around use of unsafe. We should consider carefully how much our use of unsafe is actually worth, i.e. whether saving an allocation or avoiding normal reflection in a loop is actually sufficiently valuable to outweigh the inherent risk in our approach.

We should also seriously consider using community-supported options, such as https://pkg.go.dev/github.com/jmoiron/sqlx, over our home-grown alternatives.

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

No branches or pull requests

1 participant