gin-contrib / timeout

Timeout middleware for Gin
MIT License
185 stars 38 forks source link

The operation of "timeout" should be removed because it may lead to multiple handlers using the same gin.Context simultaneously, thus causing incorrect response results. #67

Open skyterra opened 6 months ago

skyterra commented 6 months ago
// New wraps a handler and aborts the process of the handler if the timeout is reached
func New(opts ...Option) gin.HandlerFunc {
    t := &Timeout{
        timeout:  defaultTimeout,
        handler:  nil,
        response: defaultResponse,
    }

    // Loop through each option
    for _, opt := range opts {
        if opt == nil {
            panic("timeout Option not be nil")
        }

        // Call the option giving the instantiated
        opt(t)
    }

    if t.timeout <= 0 {
        return t.handler
    }

    bufPool = &BufferPool{}

    return func(c *gin.Context) {
        finish := make(chan struct{}, 1)
        panicChan := make(chan interface{}, 1)

        w := c.Writer
        buffer := bufPool.Get()
        tw := NewWriter(w, buffer)
        c.Writer = tw
        buffer.Reset()

        go func() {
            defer func() {
                if p := recover(); p != nil {
                    panicChan <- p
                }
            }()
            t.handler(c)
            finish <- struct{}{}
        }()

        select {
        case p := <-panicChan:
            tw.FreeBuffer()
            c.Writer = w
            panic(p)

        case <-finish:
            c.Next()
            tw.mu.Lock()
            defer tw.mu.Unlock()
            dst := tw.ResponseWriter.Header()
            for k, vv := range tw.Header() {
                dst[k] = vv
            }

            if _, err := tw.ResponseWriter.Write(buffer.Bytes()); err != nil {
                panic(err)
            }
            tw.FreeBuffer()
            bufPool.Put(buffer)

        case <-time.After(t.timeout): 
            c.Abort()
            tw.mu.Lock()
            defer tw.mu.Unlock()
            tw.timeout = true
            tw.FreeBuffer()
            bufPool.Put(buffer)

            c.Writer = w
            t.response(c)
            c.Writer = tw
        }
    }
}

case <-time.After(t.timeout): c.Abort()

If a timeout occurs and c.Abort() is called here, but the handler function has not completed execution (for example, waiting for database data to be read), it will still continue to execute. In such a case, when c.PureJSON() is called in the handler function, c may have already been used by subsequent requests.

so, i suggest remove timeout operation.

willthrom commented 1 month ago

Isn't this a serious issue?

We are having a weird situation where this might be the issue. We end with a request being responded with TWO json, where the first JSON seems to come from a previous request.

I can say thought the first request failed with a context cancel, and the second request when through successfully but the final response JSON seems to have, as mentioned, both message!

I don't see though how come the context could be shared between two request (I might not seeing in the library code..), even thought that seems exactly what it is happening!