An interesting mistake with Go's context package

https://utcc.utoronto.ca/~cks/space/blog/programming/GoContextValueMistake

by goranmoomin • 6 years ago

70 72 6 years ago

utcc.utoronto.ca

gravypod2• 6 years ago

Very interesting article. I've run into similar issues with Contexts (in gRPC + Java). It's been so bad they I've had to hide all interaction with contexts behind interfaces that only allow down stream users to call `getThing()` to pull things out of the contexts.

Contexts seem like exceptions but backwards and with less syntactic sugar which feels like an extremely annoying design clash with golang's error handling semantics. It seems like a better way of handling this would be some dynamic thread/context aware DI system but that's also not very go-y.

Does anyone have experience taming complexity here around this area?

neilalexander• 6 years ago

Mostly I think of contexts as a way of helping you to deal with the fact that something might initiate some work but that you can’t necessarily guarantee that you’ll be given enough time to complete it. This might be because there’s a time limit/deadline or because some other condition can be cancelled. This is useful particularly in places where you are dealing with clients/servers over a network.

An example of this is accepting an HTTP request from a client and then creating a response to that request. Ultimately, the client can ultimately give up waiting at any time and disconnect, so being given a request context allows you to deal with this situation - you can check if the context is cancelled at any time and abort whatever expensive operation you were midway through doing or about to do.

Some internal packages, such as net/http and database/sql, accept contexts as a means of avoiding unnecessary work - they will just “no op” if the context has been cancelled by that point, or they may add some new information to the context and pass it downward.

You can also check yourself if a context has been cancelled and you can even use the Done() function to break out of a “select” block early.

Another important detail is that contexts are always limited by their parent context. If the parent gets cancelled, so will any child contexts (such as those created using WithTimeout or WithCancellation).

georgethomas• 6 years ago

I've experienced both: explicit passing of immutable Contexts in Go and using ThreadLocal storage in Java (DropWizard) apps.

I think it was our mistake to use ThreadLocal storage for request-specific data. This directly couples threads (which are an OS / scheduling concern) to the serving of requests. We then ran into issues spawning new background threads, or naively switching to Kotlin Coroutines (which can be scheduled on a different thread).

So I prefer the explicit, immutable context passing that Go uses. The `getThing()` pattern you mention seems reasonable and gives you some type safety that is otherwise missing with a bare `context.Value()` calls.

gravypod2• 6 years ago

I think golang is in a unique position to make `ThreadLocal`-like communication-by-state a workable solution due to the go-routine process model. You can get the benefits of imitable-ness + type safety of `ThreadLocal`-ness with some simple maneuvering.

This could be accomplished by treating "context" like in-process environment variables. Essentially:

1. My Threadlet Variables are immutable to me.

2. I can create children Threadlets (by default) inherit all of my Threadlet Variables.

3. I can modify the Threadlet Variables of my children Threadlets.

This could look something like this (Java + Golang mashup):

    class User {
        ...
    }

    class UserSingleton implements ThreadletLocal {
         // implements static set() and static get() and reset() or something
    }

    requestMiddleware(request, next) {
        go next(request), Context.with(
           UserSingleton.set(extractUserFrom(request.headers)),
           DeadlineThreadletLocal.within(1 second)
        );
    }

This idea is much more refined and further developed in Laravel's facade APIs. They can bind an object dynamically to a "class" and change that underlying implementation dynamically. The benefit of this approach is you can actively distinguish between: 1. who is using your the User class and 2. who is injecting/receiving a User class statically. This allows you to automatically track your "DI". You could also make some syntactic sugar for this like:

    class UserSettingsController {
       @ProvidedBy(UserSingleton.class)
       Optional<User> user;

       .... some code here who talks to user ....
    }

From a Java perspective I think this should be possible post-Loom.

cle2• 6 years ago

I disagree that they’re like exceptions. Sure they share state but that’s not a core characteristic of exceptions. Contexts leave it up the consumer to decide when to care about state changes, something exceptions definitely do not.

Contexts are complicated because concurrency is complicated. You can translate that complexity into other complexity like your Java example (to keep code idiomatic and unsurprising), but the intrinsic complexity doesn’t really go away. You can build higher level abstractions that add constraints on what you can do (eg structured concurrency) in order to hide the complexity, but that’s at a different abstraction layer than contexts.

sudhirj• 6 years ago

One of the things I miss in Go is immutability. Here is the context object is trying to be kept immutable with the pattern of returning a copy of the object with the mutation, which is pretty great but needs to be popular in the ecosystem to use it effectively.

cle2• 6 years ago

It’s not about immutability, it’s about cascading contexts, all the Context “With” functions create children that are automatically cancelled when any ancestor is cancelled.

morelisp8• 6 years ago

It is about immutability, at least in the visible API. The stdlib context functions all only expose the `context.Context` interface which has no methods to mutate it. Any "changes" to a context can only be done by making a child context, which also ensures the caller's desired deadline/timeout semantics are enforced as each layer can only add cancellation conditions; similarly you cannot remove values, only replace them.

(Though internally there are some nasty optimizations to chain cancellation with minimal channel waits.)

zmj2• 6 years ago

Aren't contexts supposed to flow mutations down the call stack, not up it?

nkozyra• 6 years ago

That was my first impression and why I guessed nil value.

eternalban• 6 years ago

And if the functions in question were properly named

   func NewWithValue(parent Context, key, val interface{}) Context

   func NewBackground() Context

you wouldn't even have to guess.

travmatt• 6 years ago

My second thought was that go will pass the struct by value, so even if it didn’t crash that member would not have been set in the parent function.

mperham2• 6 years ago

The Context package is tricky to use correctly. I got bit because I designed an API that assumed you could embed a Context within a larger struct. Because it's immutable you can't and have to pass it as a value everywhere. It wasn't fun having to break backwards compatibility to fix that mistake.

AlexCoventry• 6 years ago

Not sure what you mean by "embed a Context within a larger struct." It's certainly possible to have a struct field of type context.Context.

sagichmal• 6 years ago

Possible, but usually a bad idea.

mperham2• 6 years ago

I didn’t say it wasn’t possible, I meant could as in “safe to do so”. Read the package docs, it explicitly says not to do that.

Nullabillity3• 6 years ago

At the risk of sounding like a broken record, Rust attacks this problem from multiple avenues...

1. You can tag a type or function as #[must_use], either of which makes discarding a value trigger a warning. This would have caught the (equivalent of the) example code.

2. Futures provide pretty much everything that Context does, but without having to thread a magical value correctly through everything. Explicit cancellation can be had just by.. dropping the Future. This means that you can get timeouts by racing with a timer Future. Arbitrary values (as in the example) are slightly more verbose, but you can have a decorator Future that sets a thread-local for the duration of the poll.

Thaxll• 6 years ago

If don't find decorator / tags in programming elegant, it polutes your code so much.

oh_sigh2• 6 years ago

How would you like to tag functions or things with properties (like the "return value must be used" property?

networkimprov4• 6 years ago

Rust also permits termination of blocking syscalls.

Once upon a time it automatically retried many syscalls after EINTR, but it dropped that design flaw.

papaf• 6 years ago

Explicit cancellation can be had just by.. dropping the Future.

Its great to have a Rust expert to turn up in a Go thread to educate the masses.

I have one question from the cheap seats though. How do you drop the Future, with a decision made at runtime, given that lifetimes are part of the type system?

Thanks for your help.

Nullabillity3• 6 years ago

It's not very useful to borrow futures, so that tends not to be an issue in practice.

msbarnett• 6 years ago

I have one question from the cheap seats though. How do you drop the Future, with a decision made at runtime, given that lifetimes are part of the type system?

Trivially, something like

    let future = some_long_running_async_function();
    let timer = delay(Duration::from_millis(10000));

    select! {
        result = future => {
          // handle result of future completing successfully
        },
        _ = timer => {
            // future missed the deadline, drop it, terminating execution.
            std::mem::drop(future);
        }
    }

If this were wrapped in a function you could also just return out of the timer branch and let the future naturally drop as the stack popped.

clktmr• 6 years ago

Futures provide pretty much everything that Context does, but without having to thread a magical value correctly through everything.

It has downsides though:

* Instead you have to use async/await throughout your code and suddenly have two execution models that you need to reason about

* Dropping the future won't help if the call is hanging on a blocking call either

* The callee doesn't know anything about a possible deadline

* You can only handle the cancellation in the callee in `Drop`, which gets complicated quickly if you need to rollback changes, because you need to track state manually

* After all the use of 'context' is optional. You only need to pass it if you need cancellation, whereas in async Rust you always need to keep in mind that you might get canceled.

cfors• 6 years ago

Go's context package is great, but also hard to reason about. As developers, we often times like to think that just piping a context through a function chain will take care of all of your timeouts concerns for you.

Some fun context gotchas:

- Using `req, err := http.NewRequest(...)` to create an http request, and then calling `client.Do(req)` does not do anything with context, you need to use `http.NewRequestWithContext(...)`. [0]

- Creating a function that accepts a `ctx context.Context` and then making a blocking call that doesn't respect your context will still block for the entirety unless you instrument it yourself.

- If a parent context is canceled, all children (derived) contexts will also be canceled. However, only if they are explicitly checking `context.Done()`.

- And the worst of all is passing things that decide business logic in your context variables! First, you lose all benefits of go's static typing. Second, you will not realize all of the crazy places that a context scoped variable is coming from, have fun debugging that!

[0] https://golang.org/pkg/net/http/#NewRequest

bigdubs4• 6 years ago

The issue here is contexts were introduced after the API for `http.NewRequest()` was guaranteed to be stable, so there are in effect (2) different versions of the api, one without contexts and one with.

IMHO it's a rock and a hard place; either you break compatibility, or you have a slightly more confusing API.

mook• 6 years ago

At least having a deprecation warning in the docs would be nice. Doing it compile time doesn't seem like the kind of thing golang would do; that seems like a thing external linters might have in that ecosystem (and likely already does).

erik_seaberg2• 6 years ago

Or you add first-class context handling to the language, because this won't be the last time and continually adding more plumbing to every method signature is unsustainable.

networkimprov4• 6 years ago

Most blocking syscalls in Go cannot be terminated by any means. [1]

It seems unlikely that the stdlib os API will add dozens of variants taking context.Context. The new FS API proposal makes no mention of deadlines or contexts. [2]

[1] https://github.com/golang/go/issues/41054

[2] https://github.com/golang/go/issues/5636#issuecomment-661926...

ohnoesjmr2• 6 years ago

Atleast file io used to spawn a thread just for that, so cancellations and deadlines were still respected

morelisp8• 6 years ago

The dual purposes of contexts (cancellation vs. value propagation) is one of the biggest warts in the go stdlib. It really feels like a right hand / left hand situation within Google.

They are a pretty good tool for handling cancellation. They are abysmal at carrying values - no type safety, slow, and as the documentation notes require extreme care in key types. The type safety is probably foregone given the constraints of Go's type system, but the other two are direct consequences of marrying the values to the same interface requirements (additive only, mandatory ancestors) as cancellation, which are too weak for a good kv map.

We simply don't use the value features unless we're using one of the HTTP routers that forces us to, and then only for the router's parameters.

zmj2• 6 years ago

The commonality is that both should flow across serialization boundaries, like RPC calls to downstream services.

kevindong• 6 years ago

Creating a function that accepts a `ctx context.Context` and then making a blocking call that doesn't respect your context will still block for the entirety unless you instrument it yourself.

A Golang service my work team owns has this problem. Virtually every single endpoint the service has contains a context as the first parameter... but the service never actually uses the context in the myriad of endpoints the service has.

jrockway2• 6 years ago

The big problem is that contexts are mostly mandatory if you want to be able to reason about the number of goroutines that are going to be around concurrently, but that the language syntax favors forgetting about them. For example, something like:

   go func() { ch <- foo } ()

Can easily block forever if you are no longer reading ch. Instead, you pretty much have to guard all channel reads and writes with:

   select {
   case <-ctx.Done():
       return ctx.Err()
   case ch <- foo:
   }

To some extent, this is all perfectly fair. Any programmer should see the first example and think "what causes this to return?" and won't be surprised when their program crashes every 2 weeks because it runs out of memory. But... it surprises everyone. I bet it might be the number one problem with Go, actually.

The side effect of not planning to abort operations that will never complete is that people are super okay with using locking machinery that can't be aborted. You will see all sorts of things like WaitGroups or Mutexes inside operations that have an externally-limited lifespan, and then you get the same problem of running out of memory because you have a billion things waiting on some lock; waiting for their turn to do a computation and then discard the result because the caller disappeared long ago. Feels bad. (My number one Go pet peeve, btw, is mixing different types of locking machinery. Everyone should use channels pretty much 99.9% of the time, but should never lock a mutex before doing a channel write, or something... which I've seen.)

I assume the expectation is that "something else" is supposed to make this all work. That's how it works in UNIX land. How does cat know to exit after printing 10 lines in a pipeline like "cat ... | head"? Easy; the OS kills it when head closes stdin. Unfortunately, nothing is sitting around killing your goroutines when someone presses the "stop" button in their browser. That is up to the application developer, i.e. you.

morelisp8• 6 years ago

Everyone should use channels pretty much 99.9% of the time

This is the kind of thing that sounds nice in theory, but is not borne out as a good idea by research into what actually causes errors[0] nor is it how the standard library is actually implemented (context iself[1] mixes mutexes and channels liberally, you'll find similar code in http.Client and http.Server, sql.Tx and sql.DB, etc.).

It is technically correct that you do not need to lock anything before sending a message down a channel. In practice, performance reasons and the anemic channel API means you often mix both.

[0] https://blog.acolyer.org/2019/05/17/understanding-real-world... [1] https://golang.org/src/context/context.go

throw_m239339• 6 years ago

If the Go team made Goroutines types like ADA tasks, nobody would have to deal with any of the problems people using Goroutines have to deal with. This is yet another short sighted design decision made by Go designers. There would be no need for a context package at first place. The Go team tried once again to solve a language design problem with a library, it didn't work.

masklinn3• 6 years ago

Could you explain how Ada tasks work and why they solve the issues the context package is intended for, for those who don't know ada?

a1369209993• 6 years ago

Seconded, that sounds interesting and a brief search didn't turn up anything promising.

johnisgood• 6 years ago

I do not know if this is anything close to what you want, but:

https://news.ycombinator.com/item?id=19245898

and

https://news.ycombinator.com/item?id=23609499

might be of use.

networkimprov4• 6 years ago

Contexts are generally regarded as a workaround for the fact that Go doesn't offer goroutine-local storage -- a design feature that's often criticized.

ohnoesjmr2• 6 years ago

Goroutine local storage is the stack? If you don't want to push values down as arguments and want some global state to access it, your code is probably horrible already. Pushing down a context as a value container instead of actual values is horrible too.

FridgeSeal• 6 years ago

If the Go team made Goroutines types like ADA tasks, nobody would have to deal with any of the problems people using Goroutines have to deal with

This generalises nicely to "If Go had paid attention to literally any lesson we've learnt about programming languages since approximately the 80's".

kevincox2• 6 years ago

Come on they got some things right, like lexical scoping and functions.

earthboundkid2• 6 years ago

Chris’s blog is general quite good, but I thought this entry was weak. I guess he doesn’t use context much for his devops stuff.

If you use context at all, you know that it doesn’t mutate in place. That’s why it’s goroutine safe. It’s the whole point of using context. The answer to the pop quiz was obvious, and I only second guessed myself because I thought it must be a trick question somehow. Like “well obviously you can’t mutate a context but maybe context.TODO is broken somehow”.

shirro• 6 years ago

For a language which seems to have been intended initially for writing maintainable network services the absence of a means to set timeouts or cancel requests in the original network apis was clearly an oversight. I like what they have proposed with generics and other recent changes and I would like to see a fresh look at this problem.

Context is working as intended here as it returns a copy which is well documented. Cancellation and timeouts are useful but WithValue seems like a nasty hack to me. I think it should be avoided and possibly deprecated.

dvt• 6 years ago

This is actually kind of funny (and true) as I was the one that implemented sensible HTTP timeouts in the standard library after a server I wrote oddly kept eternally open-connections[1].

[1] https://github.com/golang/go/issues/213

shorsher• 6 years ago

Here's a question from someone who is recently out of school and somewhat new to Go. Is it OK to use Context for request-scoped values? i.e. passing data between middleware? I have started doing this, but is there a better alternative?

Peter Bourgon's blog post[0] about context made me think it is fine. After reading the comments here, I am not so sure. Especially since his post was roughly 4 years ago.

0: https://peter.bourgon.org/blog/2016/07/11/context.html

ChrisFoster• 6 years ago

I liked the following blog post where Jack Lindamood argues pretty convincingly against using Context.Value in nearly all circumstances, and gives some nice alternatives.

https://medium.com/@cep21/how-to-correctly-use-context-conte...

An interesting mistake with Go's context package

Related Stories