Go: database/sql: context cancellation allows statements to execute after rollback [1.14 backport]

[gopherbot]

@leighmcculloch requested issue #34775 to be considered for backport to the next 1.14 minor release.

@odeke-em I believe this fix satisfies the requirements for a backport according to the minor version release policy because the issue is a serious issue that is causing databases to get into bad state where no work around is possible and updating to the latest release Go 1.14.3 does not fix the issue.

If patching Go is the suggested work around that suggests there is no work around in an application. Unfortunately building custom Go isn't an option for everyone as our product is open source and so we'd need to ask everyone who uses it to build with a custom Go.

@gopherbot please consider this for backport because this issue breaks production applications resulting in unexpected database state and cannot be fixed with a work around including updating the version of Go to the latest version.

[andybons]

Approved

[leighmcculloch]

@andybons Thanks for approving the backport 🎉. I only need this fixed in Go 1.14 but given 1.13 is still a supported Go version should I also open a backport issue for this for Go 1.13?

[andybons]

@leighmcculloch up to you. If the backport CL applies cleanly or otherwise without too much fuss then go for it.

[odeke-em]

The CLs don't apply cleanly, there are some merge conflicts with Go1.14, but also the 3 CLs were mailed in a relation chain, so that's tricky too. @kardianos could you please help with composing the backport CLs? Thank you.

[kardianos]

The only way to apply this would likely be to pull the full chain with it, or devise a new patch with the same effect. This fix relied on another fix prior to it I believe.

[kardianos]

On a desktop now. If you do a backport, you should cherry pick both https://go-review.googlesource.com/c/go/+/216197 and https://go-review.googlesource.com/c/go/+/216240 . These won't apply cleanly because they were after the change that fixes the session resetter.

[fednerjuste]

Has this change been released in 1.14

[brbaker]

Will this make it into the announced release of 1.14.5 on July 14, 2020?

[crbraun]

Surprising to me that this critical issue has been around for so long and not fixed. To not be able to rely on SQL transaction ACID rules is very dangerous indeed. Am I to assume that developers are not using auto commit or not building mission critical applications that require SQL transactions to be atomic in nature?

I encourage the community to back port this fix as soon as possible. Thanks for listening.

Chris

[ianlancetaylor]

@brbaker Nobody has done the backport yet. Also 1.14.5 is going to be a security release so this will definitely not be in that release. The question is whether somebody does the backport to get it into the 1.14.6 release.

[gopherbot]

Change https://golang.org/cl/242101 mentions this issue: [release-branch.go1.14] database/sql: backport 3 Tx rollback related CLs

[gopherbot]

Change https://golang.org/cl/242102 mentions this issue: [release-branch.go1.14] database/sql: backport 3 Tx rollback related CLs

[odeke-em]

Hey folks, sorry for the delay. It was quite a thorny cherry pick because:
a) A relation chain in 3 CLs that had merge conflcits
b) I don't have the Gerrit "forge-author" permission set
thus the normal way of making cherry-picks was a pain.

However, due to the attention this has gotten I was able to sit down and manually
make those cherry picks, along with some git magic and I've mailed https://go-review.googlesource.com/c/go/+/242102
for Go1.14.X, so at least now we have a cherry pick up :)

[romandvoskin]

@odeke-em Thank you! Does this mean that with the cherry pick, one can manually build a custom distribution by applying the cherry pick on the latest 1.14 release? I'm asking because it seems 1.14.5 is deemed a security release and will not include the SQL fixes and we can't wait for an official release to fix this critical bug in our organization.

[odeke-em]

Yes, indeed that’ll work if you get the lone composite CL
https://go-review.googlesource.com/c/go/+/242102

This issue was approved for backporting to Go1.14.5 (has been for quite a
while) but we’ll see whether the release team accepts it or not. If it is
drawn for the release of Go1.14.5, you’ll be in luck to use it ASAP without
manually building Go yourself, or might just have to wait until Go1.14.6 is
released.

On Sun, Jul 12, 2020 at 5:28 AM Roman Dvoskin [e-mail]
wrote:

@odeke-em https://github.com/odeke-em Thank you! Does this mean that
with the cherry pick, one can manually build a custom distribution by
applying the cherry pick on the latest 1.14 release?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/39101#issuecomment-657215428, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ABFL3V47E6LDEJK77KWAYQTR3GT75ANCNFSM4NBYLTSQ
.

[andybons]

We are waiting for the 1.13 backport CL to be reviewed before either can land in a point release. We are planning to cut tomorrow, and if it doesn’t make it in, then we can aim for the August 1 point release in two weeks.

[kardianos]

@andybons I've reviewed the 1.13 backport CL with a +2. Good to go from my end.

[gopherbot]

Closed by merging 399ce807381f897d5ea7bdd3970017976d249738 to release-branch.go1.14.

[andybons]

Thank you very much for the quick review, @kardianos.