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

database/sql/driver: TestTxStmtDeadlock very flaky on android-amd64-emu #46916

Closed
mdempsky opened this issue Jun 24, 2021 · 6 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Soon This needs to be done soon. (regressions, serious bugs, outages)
Milestone

Comments

@mdempsky mdempsky added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Soon This needs to be done soon. (regressions, serious bugs, outages) labels Jun 24, 2021
@mdempsky mdempsky added this to the Go1.18 milestone Jun 24, 2021
@cuonglm
Copy link
Member

cuonglm commented Jun 25, 2021

I can reproduce it locally on Macbook M1:

$ go test -race -count=100 -run=TestTxStmtDeadlock
--- FAIL: TestTxStmtDeadlock (0.00s)
    sql_test.go:2851: sql: transaction has already been committed or rolled back
--- FAIL: TestTxStmtDeadlock (0.00s)
    sql_test.go:154: Error closing fakeConn: fakedb: can't close; dangling statement(s)
    sql_test.go:172: error closing DB: fakedb: can't close; dangling statement(s)
FAIL
exit status 1
FAIL	database/sql	0.934s

The fix in b73cc4b seems make the cancel() call too early, moving the call after tx.Prepare will fix the problem.

@cuonglm
Copy link
Member

cuonglm commented Jun 25, 2021

@mdempsky oh, it's fixed in 44f9a35, seems we just need to merge master to dev.typeparams

@mdempsky
Copy link
Member Author

@cuonglm Oh nice, thanks for tracking that down. Do you want to send a merge CL? I'm available to review/approve.

@cuonglm
Copy link
Member

cuonglm commented Jun 25, 2021

@cuonglm Oh nice, thanks for tracking that down. Do you want to send a merge CL? I'm available to review/approve.

I'm on it, need to resolving conflicts in internal/buildcfg/exp.go

@bcmills
Copy link
Member

bcmills commented Jun 25, 2021

Duplicate of #46852

@bcmills bcmills marked this as a duplicate of #46852 Jun 25, 2021
@cuonglm
Copy link
Member

cuonglm commented Jun 28, 2021

@cuonglm Oh nice, thanks for tracking that down. Do you want to send a merge CL? I'm available to review/approve.

https://go-review.googlesource.com/c/go/+/330829 was merged, we can now close this issue.

@cuonglm cuonglm closed this as completed Jun 28, 2021
@golang golang locked and limited conversation to collaborators Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Soon This needs to be done soon. (regressions, serious bugs, outages)
Projects
None yet
Development

No branches or pull requests

4 participants