Please sign in to comment.
context: define behavior for Err before Done is closed
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 <firstname.lastname@example.org> Reviewed-by: Sameer Ajmani <email@example.com>
- Loading branch information...