Skip to content

Commit

Permalink
context: prevent creation of invalid contexts
Browse files Browse the repository at this point in the history
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
  • Loading branch information
knusbaum authored and ianlancetaylor committed Mar 18, 2020
1 parent 61ce82a commit 0205790
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ type CancelFunc func()
// Canceling this context releases resources associated with it, so code should
// call cancel as soon as the operations running in this Context complete.
func WithCancel(parent Context) (ctx Context, cancel CancelFunc) {
if parent == nil {
panic("cannot create context from nil parent")
}
c := newCancelCtx(parent)
propagateCancel(parent, &c)
return &c, func() { c.cancel(true, Canceled) }
Expand Down Expand Up @@ -425,6 +428,9 @@ func (c *cancelCtx) cancel(removeFromParent bool, err error) {
// Canceling this context releases resources associated with it, so code should
// call cancel as soon as the operations running in this Context complete.
func WithDeadline(parent Context, d time.Time) (Context, CancelFunc) {
if parent == nil {
panic("cannot create context from nil parent")
}
if cur, ok := parent.Deadline(); ok && cur.Before(d) {
// The current deadline is already sooner than the new one.
return WithCancel(parent)
Expand Down Expand Up @@ -511,6 +517,9 @@ func WithTimeout(parent Context, timeout time.Duration) (Context, CancelFunc) {
// struct{}. Alternatively, exported context key variables' static
// type should be a pointer or interface.
func WithValue(parent Context, key, val interface{}) Context {
if parent == nil {
panic("cannot create context from nil parent")
}
if key == nil {
panic("nil key")
}
Expand Down
15 changes: 15 additions & 0 deletions src/context/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,21 @@ func XTestWithValueChecksKey(t testingT) {
}
}

func XTestInvalidDerivedFail(t testingT) {
panicVal := recoveredValue(func() { WithCancel(nil) })
if panicVal == nil {
t.Error("expected panic")
}
panicVal = recoveredValue(func() { WithDeadline(nil, time.Now().Add(shortDuration)) })
if panicVal == nil {
t.Error("expected panic")
}
panicVal = recoveredValue(func() { WithValue(nil, "foo", "bar") })
if panicVal == nil {
t.Error("expected panic")
}
}

func recoveredValue(fn func()) (v interface{}) {
defer func() { v = recover() }()
fn()
Expand Down
1 change: 1 addition & 0 deletions src/context/x_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@ func TestLayersTimeout(t *testing.T) { XTestLayersTimeout(t) }
func TestCancelRemoves(t *testing.T) { XTestCancelRemoves(t) }
func TestWithCancelCanceledParent(t *testing.T) { XTestWithCancelCanceledParent(t) }
func TestWithValueChecksKey(t *testing.T) { XTestWithValueChecksKey(t) }
func TestInvalidDerivedFail(t *testing.T) { XTestInvalidDerivedFail(t) }
func TestDeadlineExceededSupportsTimeout(t *testing.T) { XTestDeadlineExceededSupportsTimeout(t) }
func TestCustomContextGoroutines(t *testing.T) { XTestCustomContextGoroutines(t) }

0 comments on commit 0205790

Please sign in to comment.