How to do a cleanup of context cancelling?

[Jingguo Yao]

https://blog.golang.org/context uses the following code to cancel the request if ctx.Done is closed before the goroutine exits:

func httpDo(ctx context.Context, req *http.Request, f func(*http.Response, error) error) error {

    // Run the HTTP request in a goroutine and pass the response to f.

    c := make(chan error, 1)

    req = req.WithContext(ctx)

    go func() { c <- f(http.DefaultClient.Do(req)) }()

    select {

    case <-ctx.Done():

        <-c // Wait for f to return.

        return ctx.Err()

    case err := <-c:

        return err

    }

}

I have two questions about this piece of code.

First, if neither "http.DefaultClient.Do" nor "f" does a check of "ctx.Done()", "f(http.DefaultClient.Do(req))" can't be  cancelled. It means that if I want to use "ctx.Done()" to cancel some functions, I need to check "ctx.Done()" at various points in the functions which I want to cancel. Is my understanding correct?

Second, if I don't want to do a cleanup of "http.DefaultClient.Do" and "f", I can just remove "<-c // Wait for f to return.". If I do this, there will be a dangling running goroutine after httpDo returns. Is this approached considered bad?

[Ian Lance Taylor]

On Thu, Feb 14, 2019 at 2:13 AM Jingguo Yao <yaoji...@gmail.com> wrote:
>
> https://blog.golang.org/context uses the following code to cancel the request if ctx.Done is closed before the goroutine exits:
>
> func httpDo(ctx context.Context, req *http.Request, f func(*http.Response, error) error) error {
> // Run the HTTP request in a goroutine and pass the response to f.
> c := make(chan error, 1)
> req = req.WithContext(ctx)
> go func() { c <- f(http.DefaultClient.Do(req)) }()
> select {
> case <-ctx.Done():
> <-c // Wait for f to return.
> return ctx.Err()
> case err := <-c:
> return err
> }
> }
>
> I have two questions about this piece of code.
>
> First, if neither "http.DefaultClient.Do" nor "f" does a check of "ctx.Done()", "f(http.DefaultClient.Do(req))" can't be cancelled. It means that if I want to use "ctx.Done()" to cancel some functions, I need to check "ctx.Done()" at various points in the functions which I want to cancel. Is my understanding correct?

Yes.

> Second, if I don't want to do a cleanup of "http.DefaultClient.Do" and "f", I can just remove "<-c // Wait for f to return.". If I do this, there will be a dangling running goroutine after httpDo returns. Is this approached considered bad?

While there are specific cases where leaving a dangling goroutine is
harmless, as a general guideline it should be avoided. It's easy to
imagine a busy server quickly building up many thousands of dangling
goroutines, and they do have a cost.

Ian

[Jeevesh Juneja]

Proabably should change code to:

func httpDo(ctx context.Context, req \*http.Request, f func(\*http.Response, error) error) error { // Run the HTTP request in a goroutine and pass the response to f. c := make(chan error, 1) req = req.WithContext(ctx) go func() { c <- http.DefaultClient.Do(req) }() select { case <-ctx.Done(): <-c // Wait for f to return. return ctx.Err() case err := f(<-c): return err }

Then execution of f will be cancelled, if the context is already done. And we will save computation.

Is my understanding correct?

[Brian Candler]

I'm pretty sure that code won't code will compile as written. Firstly,  http.DefaultClient.Do(req) returns two values, but c is a chan error.  Secondly, the second case statement is invalid syntax (case err := f(<-c):) - you can't put a function call in here, only a send or receive.  That is, you have to receive the value in the case, and then apply f() to it in another line.

But in principle, yes you could do something like that, if you copy those values to a struct, say.