gin icon indicating copy to clipboard operation
gin copied to clipboard

Context Copy does not detach request.context

Open taehwoi opened this issue 1 year ago • 3 comments

Description

It seems like we can use context.Copy() to detach gin context from the request's scope. Or at least that is what the docs suggest, regarding using Copy() to pass context to a goroutine. However the Copy() method reuses the pointer to c.Request, so the copied context is still cancelled by original request's cancellation.

I think Copy() should do something comparable to the newly added https://pkg.go.dev/context@master#WithoutCancel, preserving the values but removing deadline and cancel propagation of the parent context.

How to reproduce

package main

import (
        "fmt"
        "time"

	"github.com/gin-gonic/gin"
)

func main() {
	g := gin.Default()
        g.ContextWithFallback = true
	g.GET("/", func(c *gin.Context) {

		go func(c *gin.Context) {
			<-c.Done()
			fmt.Println("ctx1 cancelled")
		}(c)

		go func(c *gin.Context) {
			<-c.Done()
			fmt.Println("ctx2 cancelled")
		}(c.Copy())

                time.Sleep(time.Minute)
	})
	g.Run(":9000")
}

Expectations

$ curl -m 3 http://localhost:9000

only ctx1 cancelled should be printed

ctx1 cancelled

Actual result

$ curl -m 3 http://localhost:9000

the copied context is cancelled as well

ctx1 cancelled
ctx2 cancelled

Environment

  • go version: go1.19.3 darwin/arm64
  • gin version (or commit ref): v1.8.2
  • operating system: mac os

taehwoi avatar Apr 01 '23 02:04 taehwoi

The go.Context.Done method is implemented as follows.

// Done returns nil (chan which will wait forever) when c.Request has no Context.
func (c *Context) Done() <-chan struct{} {
	if !c.engine.ContextWithFallback || c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Done()
}

So I think you should set ContextWithFallback to false to meet expectations.

type Engine struct {
	// ....
	// ContextWithFallback enable fallback Context.Deadline(), Context.Done(), Context.Err() and Context.Value() when Context.Request.Context() is not nil.
	ContextWithFallback bool
        // ....
}

Tseian avatar Apr 02 '23 03:04 Tseian

The go.Context.Done method is implemented as follows.

// Done returns nil (chan which will wait forever) when c.Request has no Context.
func (c *Context) Done() <-chan struct{} {
	if !c.engine.ContextWithFallback || c.Request == nil || c.Request.Context() == nil {
		return nil
	}
	return c.Request.Context().Done()
}

So I think you should set ContextWithFallback to false to meet expectations.

type Engine struct {
	// ....
	// ContextWithFallback enable fallback Context.Deadline(), Context.Done(), Context.Err() and Context.Value() when Context.Request.Context() is not nil.
	ContextWithFallback bool
        // ....
}

@Tseian Thanks for the suggestion. But setting ContextWithFallback = false returns a nil channel for both c.Done()and c.Copy().Done(). So neither c, nor c.Copy() is cancelled.

I was hoping that with ContextWithFallback = true only c would be cancelled on request cancel.

taehwoi avatar Apr 02 '23 11:04 taehwoi