-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
database/sql: data race in runtime string copy #43576
Comments
Interesting! Also cc'ing your co-owners: @bradfitz @kardianos |
Author of sqlc here, very interested in following along. |
Update, I'm able to consistently reproduce this error now. |
This is my current minimal test case func getCompany(ctx context.Context, qs *db.ManagedDB, cname string) (*db.Company, error) {
group, errctx := errgroup.WithContext(ctx)
group.Go(func() error {
qs.GetControllerByHostname(errctx, cname)
return errStopRacing
})
group.Go(func() error {
qs.GetCompanyByController(errctx, cname)
return errStopRacing
})
group.Wait() // either nil or errStopRacing
return nil, nil
} I can't reproduce if i replace |
Updated minimal test case, I think this is actually to the point where it's reasonable for someone else to try to reproduce. I am running lib/pq at tip and Go at tip. sqlc is not involved anymore. func TestGetCompany(t *testing.T) {
t.Parallel()
conn := testkit.OpenDB(t)
t.Cleanup(func() { conn.Close() })
for j := 0; j < 20; j++ {
var wg sync.WaitGroup
for i := 0; i < 100; i++ {
wg.Add(1)
go func() {
defer wg.Done()
err := getCompany(conn.DB)
if err != nil {
panic(err)
}
}()
}
wg.Wait()
}
} Source code: func getCompany(qs *sql.DB) error {
group, errctx := errgroup.WithContext(context.Background())
group.Go(func() error {
var s string
qs.QueryRowContext(errctx, "SELECT 'online'::controller_status").Scan(&s)
return errStopRacing
})
group.Go(func() error {
var s string
qs.QueryRowContext(errctx, "SELECT 'online'::controller_status").Scan(&s)
return errStopRacing
})
group.Wait()
return nil
} And then in the database (Postgres), you need:
I am running with the following test invocation:
Here's the current data race stack trace.
|
I'm 95% convinced that this is an error in github.com/lib/pq. |
OK, I tracked this down to a race condition in lib/pq - basically, pq wasn't prepared for database/sql to send a That is probably a fairly common case and I'm wondering if we could add some sort of driver compatibility layer to test that out. |
I think there is a separate issue for such a test suite. I'm closing because this doesn't seem a problem for database/sql. |
I ran the company's test suite and hit a data race. Unfortunately I am having trouble reproducing it - I ran the specific failing tests 10,000 times, and could not produce the error again. I am pasting this because I am concerned about the potential for memory corruption and in the hopes that the stack trace can pinpoint the source of the problem. I'm unsure what layer it's at.
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
I can't reproduce this reliably enough to determine that its absence is proof it was fixed, see the issue summary.
What operating system and processor architecture are you using (
go env
)?go env
OutputHere is the data race stack trace:
We're using sqlc v1.6.0 as an ORM layer, and github.com/lib/pq at commit 083382b7e6fc. It's possible the error is in those layers; I can't reproduce it frequently enough, so I can't say for sure whether updating to the latest commit fixes the error. I'm not sure whether a driver is responsible for marking memory as dirty or not.
Here are some of the relevant parts of the source code:
v1_network_access.go line 163 and 186
controllers.sql.go line 248
db/models.go line 108
companies.sql.go line 152
which calls
internal/db/db.go line 1302
I can send a full version of the code privately or try out different things if you have ideas about how to reproduce it.
The text was updated successfully, but these errors were encountered: