Misleading context.WithTimeout(): Timeout shouldn't start counting once you create the context

[dcruzin]

What version of Go are you using (go version)?

go1.13.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/david.cruz/Library/Caches/go-build"
GOENV="/Users/david.cruz/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY="dummy"
GONOSUMDB="dummy"
GOOS="darwin"
GOPATH="/Users/david.cruz/Go"
GOPRIVATE="dummy"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/david.cruz/Go/src/dummy/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/c4/ny3xl46x44q_s74l7p_3555jhkqrgp/T/go-build207377976=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

ctx, cancel := context.WithTimeout(context.Background(), time.Duration(2)*time.Second)
defer cancel()
time.Sleep(3 * time.Second)
err := foo(ctx)

What did you expect to see?

There shouldn't be a "context deadline exceeded" error.

What did you see instead?

Function "foo" returned a "context deadline exceeded"

I'm not sure what you expected to happen, but the documentation for WithTimeout clearly states:

WithTimeout returns WithDeadline(parent, time.Now().Add(timeout)).

[mdlayher]

This is the way the context package works. There is no automatic way to trigger the timeout to begin when you pass the context as a parameter.

If this is the behavior you desire, create the context and set its deadline immediately before your function call.

[ianlancetaylor]

As far as I can see this is working as documented and intended, so closing the issue. Please comment if you disagree.

[dcruzin]

It is working as documented, therefore I'm not saying it's wrong.

My proposal and intention is that context should carry a duration, rather than a deadline. The responsibility to "trigger the timeout to begin" shifts to the client's library.

This will give meaning to withTimeout() function, while also making the timeout duration more assertive.

[ianlancetaylor]

We can't change WithTimeout, of course, as that would break existing code.

It sounds like you are suggesting a couple of methods

func WithStartableTimeout(context.Context, time.Duration) context.Context
func StartTimeout(context.Context)

Is that right? When would you want to use such a feature?

[dcruzin]

What about preserving the Timeout Duration in the context variable?

It doesn't break anything and libraries can decide if they want to use the Timeout Duration, instead of a Deadline.

[ianlancetaylor]

I'm sorry, I don't understand what that means in practice. Can you show what that might look like in code?

[dcruzin]

For the sake of simplicity, context.go could be something like:

...

type timerCtxTimeout struct {
	timerCtx
	timeoutDuration time.Duration
}

func (c *timerCtxTimeout) Timeout() (timeout time.Duration, ok bool) {
	return c.timeoutDuration, true
}

func WithTimeout(parent Context, timeout time.Duration) (Context, CancelFunc) {

	// TODO: Validate if there is a previous timeout/deadline here
	c := &timerCtxTimeout{
		timerCtx: newTimerCtx(parent)
		timeoutDuration: timeout,
	}
	// Same implementation as WithDeadline()
	return c, func() { c.cancel(true, Canceled) }
}

Now foo() can deal with a more accurate deadline:

func foo (ctx context.Context){
	deadline := ctx.Deadline() // The deadline which will cancel foo() once reached
	if timeout, ok := ctx.Timeout(); ok{
		// Get a more accurate deadline
		deadline = time.Now().Add(timeout)
	}
}

[ianlancetaylor]

I'm sorry, I still don't understand. The key point about a Context created with WithTimeout is when the context will be cancelled, such that the Err method return non-nil and the channel returned by the Done method is closed. In the above, when will that happen?

[dcruzin]

Here is a better solution. You'll need another function (one you already proposed 😅)

// Starts the timer in the context

// Option 1 - Reads timeout from the context. WithTimeout() must be called first
func StartTimeout(context.Context)
// Option 2 - It receives a timeout. You don't need to change WithTimeout()
func StartTimeout(context.Context, time.Duration)

Inside the StartTimeout(), timer will be something like:

c.timer = time.AfterFunc(timeout, func() {
	c.cancel(true, DeadlineExceeded)
})

In foo(), you can just call that function:

func foo (ctx context.Context){
	if _, ok := ctx.Timeout(); ok{
		context.StartTimeout(ctx) // it will be canceled when reached
	}
}

[ianlancetaylor]

I don't see why any of that has to be in the context package. You can write

    cctx, cancel := context.WithCancel(pctx)
    ...
    // Do stuff.
    ...
    // Cancel cctx when myTimeout expires.
    time.AfterFunc(myTimeout, cancel)

[dcruzin]

It has to be because Go is already committed on having a WithTimeout() function.

Don't get me wrong, what you are saying kinda makes sense!
But at the same time makes the WithTimeout() and the WithDeadline() irrelevant...

[ianlancetaylor]

The WithTimeout and WithDeadline functions are convenience functions that handle a common case. As you suggest, they are not required. But since people want them all the time, we provide them.

[dcruzin]

Someone made the decision to include a WithTimeout because it's more convenient to write WithTmeout(context, timeout) instead of WithDeadline(context, time.Now().Add(timeout)). Might as well take it off...

If I'm the only one who thinks this is both misleading and irrelevant, we can close this issue.