In CL 223019, I reduced the short timeout in the testLayers helper to
be even shorter than it was. That exposed a racy (time-dependent)
select later in the function, which failed in one of the slower
builders (android-386-emu).
Also streamline the test to make it easier to test with a very high -count flag:
- Run tests that sleep for shortDuration in parallel to reduce latency.
- Use shorter durations in examples to reduce test running time.
- Avoid mutating global state (in package math/rand) in testLayers.
After this change (but not before it),
'go test -run=TestLayersTimeout -count=100000 context' passes on my workstation.
Fixes#38161
Change-Id: Iaf4abe7ac308b2100d8828267cda9f4f8ae4be82
Reviewed-on: https://go-review.googlesource.com/c/go/+/226457
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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: 1b7dadd7db
GitHub-Pull-Request: golang/go#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>
Many of tests in this package assumed reasonable scheduling latency.
Unfortunately, scheduling latency on builders and CI systems is not
always reasonable.
Rather than expecting that a timeout is detected within a fixed short
interval, we can use (*testing.T).Deadline to portably scale the time
we're willing to wait to something appropriate to the builder.
Some of the tests also included arbitrary-duration sleeps, which are
no longer needed after CL 196521; we can remove those instead of
extending them.
Promptness of timeouts is also an important property, but testing that
property is better suited to benchmarks than to tests proper: unlike
tests, we generally expect benchmarks to be run in a quiet,
low-contention environment.
Fixes#13956
Change-Id: I0797e2267fb778c8ad94add56d797de9e2c885e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/223019
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If the parent context passed to WithCancel or WithTimeout
is a known context implementation (one created by this package),
we attach the child to the parent by editing data structures directly;
otherwise, for unknown parent implementations, we make a
goroutine that watches for the parent to finish and propagates
the cancellation.
A common problem with this scheme, before this CL, is that
users who write custom context implementations to manage
their value sets cause WithCancel/WithTimeout to start
goroutines that would have not been started before.
This CL changes the way we map a parent context back to the
underlying data structure. Instead of walking up through
known context implementations to reach the *cancelCtx,
we look up parent.Value(&cancelCtxKey) to return the
innermost *cancelCtx, which we use if it matches parent.Done().
This way, a custom context implementation wrapping a
*cancelCtx but not changing Done-ness (and not refusing
to return wrapped keys) will not require a goroutine anymore
in WithCancel/WithTimeout.
For #28728.
Change-Id: Idba2f435c81b19fe38d0dbf308458ca87c7381e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/196521
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It is unclear whether the current definition of os.IsTimeout is
desirable or not. Drop ErrTimeout for now so we can consider adding it
(or some other error) in a future release with a corrected definition.
Fixes#33411
Change-Id: I8b880da7d22afc343a08339eb5f0efd1075ecafe
Reviewed-on: https://go-review.googlesource.com/c/go/+/188758
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As discussed in
https://github.com/golang/go/issues/32463#issuecomment-506833421
the classification of deadline-based timeouts as "temporary" errors is a
historical accident. I/O timeouts used to be duration-based, so they
really were temporary--retrying a timed-out operation could succeed. Now
that they're deadline-based, timeouts aren't temporary unless you reset
the deadline.
Drop ErrTemporary from Go 1.13, since its definition is wrong. We'll
consider putting it back in Go 1.14 with a clear definition and
deprecate net.OpError.Temporary.
Fixes#32463
Change-Id: I70cda664590d8872541e17409a5780da76920891
Reviewed-on: https://go-review.googlesource.com/c/go/+/188398
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Add Unwrap methods to types which wrap an underlying error:
"encodinc/csv".ParseError
"encoding/json".MarshalerError
"net/http".transportReadFromServerError
"net".OpError
"net".DNSConfigError
"net/url".Error
"os/exec".Error
"signal/internal/pty".PtyError
"text/template".ExecError
Add os.ErrTemporary. A case could be made for putting this error
value in package net, since no exported error types in package os
include a Temporary method. However, syscall errors returned from
the os package do include this method.
Add Is methods to error types with a Timeout or Temporary method,
making errors.Is(err, os.Err{Timeout,Temporary}) equivalent to
testing the corresponding method:
"context".DeadlineExceeded
"internal/poll".TimeoutError
"net".adrinfoErrno
"net".OpError
"net".DNSError
"net/http".httpError
"net/http".tlsHandshakeTimeoutError
"net/pipe".timeoutError
"net/url".Error
Updates #30322
Updates #29934
Change-Id: I409fb20c072ea39116ebfb8c7534d493483870dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/170037
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
Though there is variation in the spelling of canceled,
cancellation is always spelled with a double l.
Reference: https://www.grammarly.com/blog/canceled-vs-cancelled/
Change-Id: I240f1a297776c8e27e74f3eca566d2bc4c856f2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/170060
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
So the net package doesn't indirectly depend on unicode tables.
But we're still not quite there, because a new test added in this CL
reveals that we still have a path to unicode via:
deps_test.go:570:
TODO(issue 30440): policy violation: net => sort => reflect => unicode
Updates #30440
Change-Id: I710c2061dfbaa8e866c92e6c824bd8df35784165
Reviewed-on: https://go-review.googlesource.com/c/go/+/169080
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make context depend on reflectlite instead of reflect in effort to
eventually make net no longer depend on unicode tables.
With this CL we're down to just:
net -> context -> fmt -> unicode tables
The next CL can remove context -> fmt.
Updates #30440
Change-Id: I7f5df15f975d9dc862c59aa8477c1cfd6ff4967e
Reviewed-on: https://go-review.googlesource.com/c/go/+/164239
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When deadline has already passed,
current context is canceled before return cancel function.
So is unnecessary to call cancel with remove from parent again
in return cancel function.
Change-Id: I37c687c57a29d9f139c7fb648ce7de69093ed623
Reviewed-on: https://go-review.googlesource.com/c/50410
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This comment has been the source of much confusion and broken dreams. We
can add it back if a tool ever gets released.
Updates #16742
Change-Id: I4b9c179b7c60274e6ff1bcb607b82029dd9a893f
Reviewed-on: https://go-review.googlesource.com/130876
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Docs of WithDeadline refers to variable "d" which does not exist
in the docs.
This commit renames the time argument to "d" to make the doc work.
Change-Id: Ifd2c1be7d2e3f7dfb21cd9bb8ff7fc5039c8d3bd
Reviewed-on: https://go-review.googlesource.com/65130
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously, the suggested code would result in the following golint warning:
“should drop = 0 from declaration of var errorsOnlyKey; it is the zero value”
Change-Id: I1a302c1e40ca89acbc76897e39097ecd04865460
Reviewed-on: https://go-review.googlesource.com/60290
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The Context definition to date has not defined what Err returns
before the Done channel is closed. Define that it returns nil,
as most implementations do.
All the standard context implementations (those in package
context and in golang.org/x/net/context) return Err() == nil
when Done is not yet closed. However, some non-standard
implementations may exist that return Err() != nil in this case,
as permitted by the Context definition before this date.
Call these "errorful implementations".
Because all the standard context implementations ensure that
Err() == nil when Done is not yet closed, clients now exist that
assume Err() != nil implies Done is closed and use calling Err
as a quick short-circuit check instead of first doing a non-blocking
receive from Done and then, if that succeeds, needing to call Err.
This assumption holds for all the standard Context implementations,
so these clients work fine in practice, even though they are making
unwarranted assumptions about the Context implementations.
Call these "technically incorrect clients".
If a technically incorrect client encounters an errorful
implementation, the client misbehaves. Because there are few
errorful implementations, over time we expect that many clients
will end up being technically incorrect without realizing it,
leading to latent, subtle bugs. If we want to eliminate these
latent, subtle bugs, there are two ways to do this:
either make errorful implementations more common
(exposing the client bugs more often) or redefine the Context
interface so that the clients are not buggy after all.
If we make errorful implementations more common, such
as by changing the standard context implementations to
return ErrNotDone instead of nil when Err is called before
Done is closed, this will shake out essentially all of the
technically incorrect clients, forcing people to find and fix
those clients during the transition to Go 1.9.
Technically this is allowed by the compatibility policy,
but we expect there are many pieces of code assuming
that Err() != nil means done, so updating will cause real pain.
If instead we disallow errorful implementations, then they
will need to be fixed as they are discovered, but the fault
will officially lie in the errorful Context implementation,
not in the clients. Technically this is disallowed by the compatibility
policy, because these errorful implementations were "correct"
in earlier versions of Go, except that they didn't work with
common client code. We expect there are hardly any errorful
implementations, so that disallowing them will be less disruptive
and more in the spirit of the compatibility policy.
This CL takes the path of expected least disruption,
narrowing the Context interface semantics and potentially
invalidating existing implementations. A survey of the
go-corpus v0.01 turned up only five Context implementations,
all trivial and none errorful (details in #19856).
We are aware of one early Context implementation inside Google,
from before even golang.org/x/net/context existed,
that is errorful. The misbehavior of an open-source library
when passed such a context is what prompted #19856.
That context implementation would be disallowed after this CL
and would need to be corrected. We are aware of no other
affected context implementations. On the other hand, a survey
of the go-corpus v0.01 turned up many instances of client
code assuming that Err() == nil implies not done yet
(details also in #19856). On balance, narrowing Context and
thereby allowing Err() == nil checks should invalidate significantly
less code than a push to flush out all the currently technically
incorrect Err() == nil checks.
If release feedback shows that we're wrong about this balance,
we can roll back this CL and try again in Go 1.10.
Fixes#19856.
Change-Id: Id45d126fac70e1fcc42d73e5a87ca1b66935b831
Reviewed-on: https://go-review.googlesource.com/40291
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Sameer Ajmani <sameer@golang.org>
It could have been defined the other way, but since the behavior has
been unspecified, this is the conservative approach for people writing
different implementations of the Context interface.
Change-Id: I7334a4c674bc2330cca6874f7cac1eb0eaea3cff
Reviewed-on: https://go-review.googlesource.com/37375
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Sameer Ajmani <sameer@golang.org>
Run-TryBot: Sameer Ajmani <sameer@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
So that testing can use context in its public API.
For #16221.
Change-Id: I6263fa7266c336c9490f20164ce79336df44a57e
Reviewed-on: https://go-review.googlesource.com/32648
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It already implemented the Timeout method,
but implementing the full net.Error is more convenient.
Fixes#14238 (again).
Change-Id: Ia87f897f0f35bcb49865e2355964049227951ca6
Reviewed-on: https://go-review.googlesource.com/30370
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Modifies context package to use map[]struct{} rather than map[]bool,
since the map is intended as a set object. Also adds Benchmarks to
the context package switching between different types of root nodes
and a tree with different depths.
Included below are bytes deltas between the old and new code, using
these benchmarks.
benchmark old bytes new bytes delta
BenchmarkContextCancelTree/depth=1/Root=Background-8 176 176 +0.00%
BenchmarkContextCancelTree/depth=1/Root=OpenCanceler-8 560 544 -2.86%
BenchmarkContextCancelTree/depth=1/Root=ClosedCanceler-8 352 352 +0.00%
BenchmarkContextCancelTree/depth=10/Root=Background-8 3632 3488 -3.96%
BenchmarkContextCancelTree/depth=10/Root=OpenCanceler-8 4016 3856 -3.98%
BenchmarkContextCancelTree/depth=10/Root=ClosedCanceler-8 1936 1936 +0.00%
BenchmarkContextCancelTree/depth=100/Root=Background-8 38192 36608 -4.15%
BenchmarkContextCancelTree/depth=100/Root=OpenCanceler-8 38576 36976 -4.15%
BenchmarkContextCancelTree/depth=100/Root=ClosedCanceler-8 17776 17776 +0.00%
BenchmarkContextCancelTree/depth=1000/Root=Background-8 383792 367808 -4.16%
BenchmarkContextCancelTree/depth=1000/Root=OpenCanceler-8 384176 368176 -4.16%
BenchmarkContextCancelTree/depth=1000/Root=ClosedCanceler-8 176176 176176 +0.00%
Change-Id: I699ad704d9f7b461214e1651d24941927315b525
Reviewed-on: https://go-review.googlesource.com/25270
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Adds a test case for calling context.WithDeadline() where the deadline
exists in the past. This change increases the code coverage of the
context package.
Change-Id: Ib486bf6157e779fafd9dab2b7364cdb5a06be36e
Reviewed-on: https://go-review.googlesource.com/25007
Reviewed-by: Sameer Ajmani <sameer@golang.org>
Run-TryBot: Sameer Ajmani <sameer@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also replace double spaces after periods with single spaces.
Change-Id: Iedaea47595c5ce64e7e8aa3a368f36d49061c555
Reviewed-on: https://go-review.googlesource.com/24431
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Some users don't realize that creating a Context with a CancelFunc
attaches a subtree to the parent, and that that subtree is not released
until the CancelFunc is called or the parent is canceled. Make this
clear early in the package docs, so that people learning about this
package have the right conceptual model.
Change-Id: I7c77a546c19c3751dd1f3a5bc827ad106dd1afbf
Reviewed-on: https://go-review.googlesource.com/24090
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: I41310ec88c889fda79d80eaf4a742a1000284f60
Reviewed-on: https://go-review.googlesource.com/23591
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Passes on OpenBSD now when running it with -count=500.
Presumably this will also fix the same problems seen on FreeBSD and
Windows.
Fixes#15158
Change-Id: I86451c901613dfa5ecff0c2ecc516527a3c011b3
Reviewed-on: https://go-review.googlesource.com/21840
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
