golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
120.1k stars 17.24k forks source link

proposal: context: add method like WithoutCancel but preserve context deadline #67135

Open danztran opened 2 weeks ago

danztran commented 2 weeks ago

Context

The current context.WithoutCancel function supported since Go 1.21 creates a new context that inherits all values from the parent except for cancellation and deadline exceeding. It loses the deadline information set on the parent. This becomes a limitation when we want to maintain a specific timeout even if the parent gets canceled.

Use case

Middleware and interceptor patterns often require preserving request deadlines. A middleware can use context.WithoutCancel to ensure its execution continues regardless of client cancellation. However, if the parent context had a deadline set, it's lost with the current implementation. This can lead to unintended behavior if the request processing takes longer than the intended timeout.

Workaround

To address this issue, I try create a function that produces a context with the same deadline as the parent, even if the parent is cancelled. Here's a workaround to the issue:

func WithoutCancelButDeadline(ctx context.Context) (context.Context, context.CancelFunc) {
  newCtx := context.WithoutCancel(ctx)
  deadline, ok := ctx.Deadline()
  if !ok {
    return context.WithCancel(newCtx)
  }
  return context.WithDeadline(newCtx, deadline)
}

This workaround has drawbacks:

Proposal new API:

A more efficient approach would be to introduce a new API that directly returns the context without these redundancies. Here are some potential names for this new function (or an alternative with a clearer name):

func WithoutCancelButDeadline(parent context.Context) context.Context
func WithoutCancelPreserveDeadline(parent context.Context) context.Context
func WithDeadlineWithoutCancel(parent context.Context) context.Context
func WithDeadlineOnly(parent context.Context) context.Context
ianlancetaylor commented 2 weeks ago

How about context.WithDeadline(context.WithoutCancel(parent), parent.Deadline()) ?

danztran commented 2 weeks ago

@ianlancetaylor

  1. context.WithDeadline(...) creates a totally new timer and cancelFunc, which is fine but both are kinda redundant in my usecase, not optimal though.
  2. parent.Deadline() returns zero time.Time and a isDeadlineSet boolean value. If deadline was not set from parent context, and boolean is not check, then the new context would be timed out immediately.
  3. There's a mistake in your suggestion that causes syntax error, parent.Deadline() returns 2 values (deadline time.Time, ok bool) for context.WithDeadline(parent context.Context, d time.Time)
ianlancetaylor commented 2 weeks ago

The point is, I think there is already a way to do what you want. If we are going to do a more convenient version, it would help to see some evidence that this is something that various different programs want to do. It's OK if unusual things are a bit harder to do.

danztran commented 2 weeks ago

Context is definitely important throughout in almost everywhere in every projects, most created variables for sure, especially when dealing with for API services. I was just curious if there is any way to make using context even more efficient as possible, to use fewer resources.
But I'm completely agree with your point though, if this is just something specific to our needs, we can find another solution that works for us.

neild commented 1 week ago

As a general point, I'd say that treating context deadlines and cancelation as separate things is always a mistake. Both bound the lifetime of an operation; continuing past the lifetime in some cases but not others likely indicates a design confusion.

Middleware which needs to perform some action after an operation's lifetime has ended probably needs to do so whether the operation ended due to cancelation or due to a timeout.

danztran commented 1 week ago

I couldn't argue with that too. In my perspective, all requests should have deadlines. Also in our usecase for some requests, we want to keep processing things even if the context gets cancelled (unexpectedly by client). That way, we wouldn't need to worry about creating new context without cancel but with timeout every time. Things like external service calls in request processing could potentially hang forever if the context is not cancelled and does not time out. If you guys think this is over engineering, please feel free to close.