Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Go 2: update context package for Go 2 #28342

Open
ianlancetaylor opened this issue Oct 23, 2018 · 22 comments

Comments

Projects
None yet
@ianlancetaylor
Copy link
Contributor

commented Oct 23, 2018

This issue is intended to cover ideas about updating the context package for Go 2.

  • The current context package leads to stuttering in declarations: ctx context.Context.
  • The current Context.WithValue function accepts values of any types, which is easy to misuse by passing, say, a string rather than a value of some package-local type.
  • The name Context is confusing to some people, since the main use of contexts is cancelation of goroutines.
  • Context values are passed everywhere explicitly, which troubles some people. Some explicitness is clearly good, but can we make it simpler?

See also #24050 and #20280.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor Author

commented Oct 23, 2018

See also #27982.

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 23, 2018

I would add to that list:

  • context.WithValue (intentionally) obscures information about the actual inputs required by a function; for example, because those inputs are required by some method of some other parameter. If we're getting real parametricity, it would be preferable to make those requirements explicit at compile time.
@OneOfOne

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

The name Context is confusing to some people, since the main use of contexts is cancelation of goroutines.

Related #28017

@josharian

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

I have struggled with performance issues in the past with package context, which were tied somewhat to the API. This led to one hack fix and contemplation of some ugly compiler changes (#18493). I’d like any re-think of context to see how lightweight we can make the package so that it can be used in very fine-grained contexts.

@mvdan

This comment has been minimized.

Copy link
Member

commented Oct 24, 2018

/cc @rogpeppe who I think had some ideas about the design of the API when it was first introduced.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2018

See also #28279.

@deanveloper

This comment has been minimized.

Copy link

commented Oct 26, 2018

Perhaps have go return a one-buffer channel. Closing the channel would cancel the goroutine, and we could have semantics similar to #27982 to check for cancellation. Reading from the channel would read the value that the function returns.

The problem with this, is that there is no way to read multiple return values, which is a pretty big issue. Not quite sure about an elegant way to solve this without having tuple types built-in to the language.

Writing to the channel would unfortunately write to the channel, but if you do stupid things, you should expect stupid behavior. It doesn't have any real use except for breaking your own code. (And we can't have it return a read-only channel if we want to be able to close it)

Context of course would still exist, but it's primary purpose would be for goroutine-local values, rather than cancellation.

@joonas-fi

This comment has been minimized.

Copy link

commented Nov 30, 2018

I feel that Context only partly solved the problem of cancellation, since there is no facility to wait until the thing we're cancelling, has been cancelled (e.g. wait for the cancelled goroutine to stop). I built a "stopper" for this use case: https://github.com/function61/gokit/blob/dc75639388d554c7f79ca7ec6c967436b6c8301c/stopper/stopper_test.go (disclaimer: I should've made "stopper" a hybrid composed of context - my design is far from good and I plan to improve it)

@deanveloper

This comment has been minimized.

Copy link

commented Nov 30, 2018

I like this comment: #21355 (comment)

A somewhat less invasive way to reduce the syntactic overhead of Context would be to make it a built-in type (like error).

That would at least cut the stutter by a third:

-func Foo(ctx context.Context) error {
+func Foo(ctx context) error {

I think my only issue with it is that functions that take contexts also typically have a Context in the function name, ie FooContext(ctx context.Context) error would become FooContext(ctx context) error, and we don't save quite as much as we think we do. Either way cutting off the redundancy is nice.

@dayfine

This comment has been minimized.

Copy link

commented Dec 15, 2018

@deanveloper any example of a FooContext function that actually have 'context' in the name? I haven't seen any of that myself. But context does spread all the way deep into call hierarchy which is sort of horrifying.

@dayfine

This comment has been minimized.

Copy link

commented Dec 15, 2018

Partially copying my reply in #20280

As someone who has been writing Go for two months. I never really understood context, or had to understand it, even though I see it everywhere and use it everywhere, as required by the code standard to pass it down in call hierarchy as the first argument. Today I am refactoring some code that uses it, and wonder if I can get it removed, so I figured out what context is for for the first time.

It is not immediate obvious for a beginner to understand the purpose context is used for, or to understand that IO would like to use context for cancellation, instead of something else, or that context is a primary mechanism of cancellation at all...

It's simply not self-explanatory enough, and I don't think the terminology is consistent for a developer who also deals with concepts with the same name in a different language. e.g. writing C++ at the same time, I would think Context has server-lifetime instead of request-lifetime.

I think that makes it worthwhile for us to carefully explain what it is to help engineers / new learners understand, and make the package itself as clear as possible. e.g. If it is for request-scoped info, why is it package not named request/context, or maybe it should be concurrency/context? Or if it is really used in conjunction with goroutine, how come it is not part of the language feature?

[The part where I don't know what I am talking about]
I feel like some of the concepts were borrowed from the web, where the functionalities, e.g. canceling an asynchronous operation might be realized by a browser API, i.e. to implement the behavior on call stack alone might not be sufficient, and require invention like this which gets a bit confusing. If that's the reality, we should state the facts very clearly and be crystal clear about the limitation and potential confusions.

@deanveloper

This comment has been minimized.

Copy link

commented Dec 15, 2018

@deanveloper any example of a FooContext function that actually have 'context' in the name? I haven't seen any of that myself. But context does spread all the way deep into call hierarchy which is sort of horrifying.

Here are a few examples off the top of my head, you can probably make a grep which could find all instances

Edit - Here's a third-party lib that I've used as well https://godoc.org/github.com/nlopes/slack

@dayfine

This comment has been minimized.

Copy link

commented Dec 15, 2018

@deanveloper oh this has to do with providing two versions of everything that either use context or not, as Go doesn't support overloading? Yeh that might need to be solved at a different level :|

@deanveloper

This comment has been minimized.

Copy link

commented Dec 15, 2018

I think not supporting overloading is actually a good thing. Makes sure that code stays readable and encourages writing multiple functions for different actions (rather than using something like a boolean flag). Although it does lead to cases like this, unfortunately.

@joonas-fi

This comment has been minimized.

Copy link

commented Dec 15, 2018

I don't think it has anything to do with overloading, rather I think it is so because the context package was introduced fairly recently, and due to Go's backwards compatibility there has to be separate mechanisms for incrementally improving code (DialContext() as in different function, or a new context field like in net/http). If the context package was there from the start, there would probably be just one net.Dial() that takes context as first arg.

@deanveloper

This comment has been minimized.

Copy link

commented Dec 15, 2018

Correct, but if Go did support function overloading, then a Dial function could have been written with a context as the first argument and not broken compatability.

It was a combination of adding features in later as well as having no support for overloading.

@dayfine

This comment has been minimized.

Copy link

commented Dec 15, 2018

@deanveloper or library writers can adopt a currying approach https://golang.org/pkg/net/http/#Request.WithContext

I also meant that having 'Foo' and 'FooContext' doesn't have to do with overloading, but it's just something library writers have to deal with, rather than something that would impact any decision regarding context itself here.

@creker

This comment has been minimized.

Copy link

commented Dec 16, 2018

@dayfine

It's simply not self-explanatory enough, and I don't think the terminology is consistent for a developer who also deals with concepts with the same name in a different language. e.g. writing C++ at the same time, I would think Context has server-lifetime instead of request-lifetime.

I agree. I think this is due to context being a combination of two completely separate concepts - WithValue aka the real "context" and cancellation. There're several blog posts written about this problem and the solution I would like Go to pursue is to split these concepts into separate types. Ideally I would like to WithValue to go away completely and leave only cancellation part of the context. After that it can be renamed to clearly state what it's for. We can look at CancellationToken from C# as an example.

Standard library have no use for WithValue at all. It should only accept cancellation tokens. Passing arbitrary values between calls might be useful but it should be limited to outside code. I don't even remember any standard library function that reads values from context. Is there any?

@acln0

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2018

@creker

I don't even remember any standard library function that reads values from context. Is there any?

net/http is one such package. It inspects client request contexts for net/http/httptrace hooks, and calls them.

@vearutop

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

If we drop WithValue from context, what could be the transport for layers of middlewares?

@deanveloper

This comment has been minimized.

Copy link

commented Jan 24, 2019

The point isn't to drop WithValue from context, but rather to split context into two separate ideas.

@zlotnika

This comment has been minimized.

Copy link

commented Mar 20, 2019

As far as I see it, context is 2 things:

  1. A cancellation mechanism
  2. A store for global variables

Now, as far as cancellation goes, I believe that really belongs somehow connected to the thing that just might get cancelled. You can always easily cancel from within a go-routine (that's just a normal error return), but, today, you have to use context to cancel from outside a go-routine. The canonical place to put a cancel function, thusly, is something like:

cancel := go doSomething()
// if we for some reason want to not do that thing, then
cancel()

Note: I would relatedly love a timeout added to sync.WaitGroup, but that's an issue for another issue.

As for a global store of variables, I think this only happened because context must be passed in to everywhere. If you really want to have a set of values that go everywhere with you, that's a nice sort of thing to construct intentionally. net/http uses the values, but it doesn't need to. It could have its own struct for this that's not awkwardly tied to other packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.