context: prevent creation of invalid contexts #37898

knusbaum · 2020-03-17T09:58:14Z

This commit makes it impossible to create derived contexts with nil parents.
Previously it was possible to create derived contexts with nil parents, and
invalid contexts could propogate through the program. Eventually this can
cause a panic downstream, which is difficult to trace back to the source
of the error.

Although WithCancel and WithDeadline already panic if parent is nil, this adds explicit checks to give a useful message in the panic.

Fixes #37908

googlebot · 2020-03-17T09:58:20Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

knusbaum · 2020-03-17T10:00:26Z

@googlebot I signed it!

googlebot · 2020-03-17T10:00:29Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

gopherbot · 2020-03-17T10:02:18Z

This PR (HEAD: f0ae60d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/223777 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2020-03-17T10:05:04Z

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

This commit makes it impossible to create derived contexts with nil parents. Previously it was possible to create derived contexts with nil parents, and invalid contexts could propagate through the program. Eventually this could cause a panic downstream, which is difficult to trace back to the source of the error.

gopherbot · 2020-03-17T10:41:39Z

This PR (HEAD: f404080) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/223777 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2020-03-17T20:48:46Z

Message from Ian Lance Taylor:

Patch Set 2: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-17T20:48:47Z

Message from Gobot Gobot:

Patch Set 2:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=fce6559c

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-17T20:52:53Z

Message from Gobot Gobot:

Patch Set 2:

Build is still in progress...
This change failed on freebsd-amd64-12_0:
See https://storage.googleapis.com/go-build-log/fce6559c/freebsd-amd64-12_0_fc0c8ddf.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-17T20:58:12Z

Message from Gobot Gobot:

Patch Set 2: TryBot-Result-1

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-17T23:05:05Z

This PR (HEAD: f051f22) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/223777 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2020-03-18T01:57:13Z

Message from Ian Lance Taylor:

Patch Set 5: Run-TryBot+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T01:57:13Z

Message from Gobot Gobot:

Patch Set 5:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=13b248c9

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T01:59:42Z

Message from Gobot Gobot:

Patch Set 5:

Build is still in progress...
This change failed on freebsd-amd64-12_0:
See https://storage.googleapis.com/go-build-log/13b248c9/freebsd-amd64-12_0_391c37e5.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T02:02:14Z

Message from Bryan C. Mills:

Patch Set 5: Code-Review+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T02:07:13Z

Message from Gobot Gobot:

Patch Set 5: TryBot-Result-1

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T02:49:40Z

This PR (HEAD: 41768f1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/223777 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2020-03-18T03:29:23Z

Message from Ian Lance Taylor:

Patch Set 7:

Does "go test context" pass on your system?

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T14:31:15Z

Message from Kyle Nusbaum:

Patch Set 7:

"go test context" passes now on my system.

Sorry, this is my first contribution and I was having trouble running the tests. I believe I have fixed them and it should all be passing now.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T15:56:23Z

Message from Ian Lance Taylor:

Patch Set 7: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T15:56:24Z

Message from Gobot Gobot:

Patch Set 7:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=06103d5f

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T16:07:36Z

Message from Gobot Gobot:

Patch Set 7: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T18:40:00Z

Message from Bryan C. Mills:

Patch Set 7: Code-Review+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T19:00:57Z

Message from Ian Lance Taylor:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T19:17:56Z

This PR (HEAD: 1b7dadd) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/223777 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2020-03-18T19:21:43Z

Message from Ian Lance Taylor:

Patch Set 8: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T19:22:58Z

Message from Gobot Gobot:

Patch Set 8:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=b52ea257

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T19:25:31Z

Message from Kyle Nusbaum:

Patch Set 8:

(1 comment)

Fixed the trailing period in all panic messages.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T19:32:47Z

Message from Gobot Gobot:

Patch Set 8: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

This commit makes it impossible to create derived contexts with nil parents. Previously it was possible to create derived contexts with nil parents, and invalid contexts could propogate through the program. Eventually this can cause a panic downstream, which is difficult to trace back to the source of the error. Although `WithCancel` and `WithDeadline` already panic if `parent` is `nil`, this adds explicit checks to give a useful message in the panic. Fixes #37908 Change-Id: I70fd01f6539c1b0da0e775fc5457e32e7075e52c GitHub-Last-Rev: 1b7dadd GitHub-Pull-Request: #37898 Reviewed-on: https://go-review.googlesource.com/c/go/+/223777 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

gopherbot · 2020-03-18T19:47:23Z

Message from Ian Lance Taylor:

Patch Set 8: Code-Review+2

Thanks.

Please don’t reply on this GitHub thread. Visit golang.org/cl/223777.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2020-03-18T19:52:01Z

This PR is being closed because golang.org/cl/223777 has been merged.

googlebot added the cla: no Used by googlebot to label PRs as having an invalid CLA. The text of this label should not change. label Mar 17, 2020

googlebot added cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. and removed cla: no Used by googlebot to label PRs as having an invalid CLA. The text of this label should not change. labels Mar 17, 2020

knusbaum marked this pull request as ready for review March 17, 2020 10:02

knusbaum force-pushed the panic-on-bad-context branch from f0ae60d to f404080 Compare March 17, 2020 10:40

context: fix new context test

f051f22

context: fix for new context tests

41768f1

context: remove trailing period from panic messages

1b7dadd

gopherbot closed this Mar 18, 2020

context: prevent creation of invalid contexts #37898

context: prevent creation of invalid contexts #37898

Uh oh!

Conversation

knusbaum commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Mar 17, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

knusbaum commented Mar 17, 2020

Uh oh!

googlebot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 17, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

gopherbot commented Mar 18, 2020

Uh oh!

Uh oh!

knusbaum commented Mar 17, 2020 •

edited

Loading