Go: context: Context.Done() is misleading, Cancelled() should be used instead

[bickerx2]

While learning Go I started working working with contexts I've found that the naming of the function that returns a Channel signaling if a context has been cancelled is misleading.

Take the following example:

package main

import (
        "context"
        "fmt"
        "time"
        "sync"
)

func fail(ctx context.Context, sleep int) {
        fmt.Println("(fail): I expect to be cancelled")
        select {
        case <-ctx.Done():
                fmt.Println("(fail): Something happen")
                return
        case <-time.After(time.Duration(sleep) * time.Second):
                fmt.Println("(fail): Something didn't happen")
                return
        }
}

func main() {
        fmt.Println("(main): A Go Context Demo")
        var blocker sync.WaitGroup
        ctx, cancel := context.WithCancel(context.Background())

        blocker.Add(1)
        go func() {
                defer blocker.Done()
                fail(ctx, 25)
        }()

        time.Sleep(4 * time.Second)
        fmt.Println("(main): Cancelling fail()")
        cancel()
        blocker.Wait()
        fmt.Println("(main): Goodbye")
}

Now have a look at this alternative, where I have replaced the Done() with Cancelled()

package main

import (
        "context"
        "fmt"
        "time"
        "sync"
)

func fail(ctx context.Context, sleep int) {
        fmt.Println("(fail): I expect to be cancelled")
        select {
        case <-ctx.Cancelled():
                fmt.Println("(fail): Something happen")
                return
        case <-time.After(time.Duration(sleep) * time.Second):
                fmt.Println("(fail): Something didn't happen")
                return
        }
}

func main() {
        fmt.Println("(main): A Go Context Demo")
        var blocker sync.WaitGroup
        ctx, cancel := context.WithCancel(context.Background())

        blocker.Add(1)
        go func() {
                defer blocker.Done()
                fail(ctx, 25)
        }()

        time.Sleep(4 * time.Second)
        fmt.Println("(main): Cancelling fail()")
        cancel()
        blocker.Wait()
        fmt.Println("(main): Goodbye")
}

I personally find the second version a lot more clear about what is going on as I no longer have to make the logic leap that when a context is "cancelled" that results in the channel that is returned via the to a function call of Done().

While it makes sense in the context of a context, as the context has been cancelled it's work has been done. This however is not as clear to people who have never used contexts before thus increasing their barrier of entry to the package (and ultimately the language as well)

[ianlancetaylor]

Note that a context can also be "Done" if it times out, if it was created using context.WithTimeout or context.WithDeadline.

That said, you may well be right that Canceled would have been a better name than Done. But I don't see any reasonable way that we can change it now.

[bickerx2]

I would agree with what you saying, however a large amount documentation and existing guides I've encountered all talk about withTimeout,withDeadline and withCancel all in the context of the Done() being called when the context has been cancelled.

Edit: Could please explain you can't see a reasonable way to change it when surely all that would be needed would be to rename Done() to Cancelled() and then re-create Done() as wrapper to Cancelled() which is deprecated?

[tv42]

@bickerx2 Context is an interface and if you add a new method to it you break all code that implements it independently.

[Sajmani]

Closing as infeasible. Per tv42's last comment, we cannot rename any methods of the Context interface while preserving the Go compatibility guarantee: https://golang.org/doc/go1compat