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

Data race in v1.2.10 #1134

Open
wavded opened this issue Feb 18, 2025 · 4 comments
Open

Data race in v1.2.10 #1134

wavded opened this issue Feb 18, 2025 · 4 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@wavded
Copy link

wavded commented Feb 18, 2025

In v1.2.9 we noticed panics when running tests that used ScanAndCount, we no longer experience that in v1.2.10 which is great, but we noticed this race condition exists in v1.2.10. This race condition does not exist in v1.2.8.

go test -race
==================
WARNING: DATA RACE
Write at 0x00c00020b558 by goroutine 47:
  github.com/uptrace/bun.(*relationJoin).applyTo()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/relation_join.go:41 +0x237
  github.com/uptrace/bun.(*SelectQuery).appendQuery.func1()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:540 +0x34
  github.com/uptrace/bun.(*SelectQuery)._forEachInlineRelJoin()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:461 +0xc2
  github.com/uptrace/bun.(*SelectQuery).forEachInlineRelJoin()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:453 +0x7e
  github.com/uptrace/bun.(*SelectQuery).appendQuery()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:539 +0x3f7
  github.com/uptrace/bun.(*SelectQuery).AppendQuery()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:513 +0xd8
  github.com/uptrace/bun.(*SelectQuery).scanResult()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:884 +0x49c
  github.com/uptrace/bun.(*SelectQuery).Scan()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:849 +0x124
  github.com/uptrace/bun.(*SelectQuery).scanAndCountConcurrently.func1()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:988 +0x125

Previous write at 0x00c00020b558 by goroutine 48:
  github.com/uptrace/bun.(*relationJoin).applyTo()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/relation_join.go:41 +0x237
  github.com/uptrace/bun.(*SelectQuery).appendQuery.func1()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:540 +0x34
  github.com/uptrace/bun.(*SelectQuery)._forEachInlineRelJoin()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:461 +0xc2
  github.com/uptrace/bun.(*SelectQuery).forEachInlineRelJoin()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:453 +0x7e
  github.com/uptrace/bun.(*SelectQuery).appendQuery()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:539 +0x3f7
  github.com/uptrace/bun.countQuery.AppendQuery()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:1308 +0x104
  github.com/uptrace/bun.(*SelectQuery).Count()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:941 +0xf3
  github.com/uptrace/bun.(*SelectQuery).scanAndCountConcurrently.func2()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:1002 +0xe4

Goroutine 47 (running) created at:
  github.com/uptrace/bun.(*SelectQuery).scanAndCountConcurrently()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:985 +0x2e4
  github.com/uptrace/bun.(*SelectQuery).ScanAndCount()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:969 +0x1e4
  github.com/my-org/my-project/postgres.(*Modeler).ActivitiesByQuery()
      /var/lib/jenkins/jobs/GH/jobs/my-project/branches/PR-15/workspace/postgres/activity.go:105 +0x498
  github.com/my-org/my-project/postgres.TestActivity.func4()
      /var/lib/jenkins/jobs/GH/jobs/my-project/branches/PR-15/workspace/postgres/activity_test.go:72 +0x194
  testing.tRunner()
      /usr/lib/go-1.23/src/testing/testing.go:1690 +0x226
  testing.(*T).Run.gowrap1()
      /usr/lib/go-1.23/src/testing/testing.go:1743 +0x44

Goroutine 48 (running) created at:
  github.com/uptrace/bun.(*SelectQuery).scanAndCountConcurrently()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:998 +0x4aa
  github.com/uptrace/bun.(*SelectQuery).ScanAndCount()
      /var/lib/jenkins/go/pkg/mod/github.com/uptrace/[email protected]/query_select.go:969 +0x1e4
  github.com/my-org/my-project/postgres.(*Modeler).ActivitiesByQuery()
      /var/lib/jenkins/jobs/GH/jobs/my-project/branches/PR-15/workspace/postgres/activity.go:105 +0x498
  github.com/my-org/my-project/postgres.TestActivity.func4()
      /var/lib/jenkins/jobs/GH/jobs/my-project/branches/PR-15/workspace/postgres/activity_test.go:72 +0x194
  testing.tRunner()
      /usr/lib/go-1.23/src/testing/testing.go:1690 +0x226
  testing.(*T).Run.gowrap1()
      /usr/lib/go-1.23/src/testing/testing.go:1743 +0x44
==================
--- FAIL: TestActivity (0.08s)
    --- FAIL: TestActivity/ActivitiesByQuery (0.02s)
        testing.go:1399: race detected during execution of test
@j2gg0s
Copy link
Collaborator

j2gg0s commented Feb 19, 2025

Sorry about that!
This looks like an existing issue before v1.2.10?
I’ll take a look at how to handle it later.

@j2gg0s j2gg0s added the bug Something isn't working label Feb 19, 2025
@wavded
Copy link
Author

wavded commented Feb 19, 2025

I "think" this issue may have existed in v1.2.9 as well but I didn't notice it because it was panicking before the race was detected but I'm not sure, seems to all stem around ScanAndCount stuff though. Thanks for looking into it @j2gg0s !

@j2gg0s j2gg0s added the help wanted Extra attention is needed label Feb 20, 2025
@j2gg0s
Copy link
Collaborator

j2gg0s commented Feb 20, 2025

Intuitive feeling, type Query and type relationJoin dont support multithreading usage,
because during usage we modify them without locking.

ScanAndCount will call scanAndCountConcurrently, increases the likelihood of competition.

I’m concerned that directly adding locks might slow down most regular queries.
We should consider the overall situation, and any help is welcome.

@Aoang
Copy link
Contributor

Aoang commented Feb 27, 2025

Could you please provide a reproducible demo?

I have some ideas, and a reproducible demo would be very helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants