Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel request by client will cause panic #2279

Open
Coding-2-86 opened this issue Mar 7, 2020 · 4 comments
Open

Cancel request by client will cause panic #2279

Coding-2-86 opened this issue Mar 7, 2020 · 4 comments

Comments

@Coding-2-86
Copy link

Coding-2-86 commented Mar 7, 2020

Description

Panic error if client cancel request when server is running a long task and response >8192 length content after it.
(content length <=8192 works fine.)

select {
	case <-r.C.Request.Context().Done():
		return
	default:
		c.String(200, strings.Repeat("A", 8193))
}

Are there any other solutions except above??

How to reproduce

package main

import (
	"github.com/gin-gonic/gin"
	"strings"
	"time"
)

func main() {
	router := gin.Default()
	router.GET("/", func(c *gin.Context) {
		time.Sleep(time.Second * 2)
		c.String(200, strings.Repeat("A", 8193))
	})
	router.Run(":8080")
}

1.Open the page directly in the browser then close the tab before server response
2.Send request in Postman then cancel it before server response

Expectations

ignore the request if it's cancelled

Actual result

GET / HTTP/1.1
Host: 127.0.0.1:8080
Connection: close
Accept: /
Accept-Encoding: gzip, deflate, br
Cache-Control: no-cache
Connection: close
Postman-Token: c137d141-a132-49ee-8087-397f5ca7d131
User-Agent: PostmanRuntime/7.22.0

write tcp 127.0.0.1:8080->127.0.0.1:54928: wsasend: An established connection was aborted by the software in your host machine.
/src/github.com/gin-gonic/gin/context.go:817 (0x8ed689)
(*Context).Render: panic(err)
/src/github.com/gin-gonic/gin/context.go:892 (0x8ff331)
(*Context).String: c.Render(code, render.String{Format: format, Data: values})
/tmp/main.go:13 (0x8ff2b5)
main.func1: c.String(200, strings.Repeat("A", 8193))
/src/github.com/gin-gonic/gin/context.go:147 (0x8e95d1)
(*Context).Next: c.handlersc.index
/src/github.com/gin-gonic/gin/recovery.go:83 (0x8fd86a)
RecoveryWithWriter.func1: c.Next()
/src/github.com/gin-gonic/gin/context.go:147 (0x8e95d1)
(*Context).Next: c.handlersc.index
/src/github.com/gin-gonic/gin/logger.go:241 (0x8fc977)
LoggerWithConfig.func1: c.Next()
/src/github.com/gin-gonic/gin/context.go:147 (0x8e95d1)
(*Context).Next: c.handlersc.index
/src/github.com/gin-gonic/gin/gin.go:411 (0x8f3b23)
(*Engine).handleHTTPRequest: c.Next()
/src/github.com/gin-gonic/gin/gin.go:369 (0x8f3214)
(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/src/net/http/server.go:2802 (0x6bab8a)
serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/src/net/http/server.go:1890 (0x6b635b)
(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/src/runtime/asm_amd64.s:1357 (0x45add0)
goexit: BYTE $0x90 // NOP

[GIN-debug] [WARNING] Headers were already written. Wanted to override status code 200 with 500

Environment

  • go version: 1.13.1
  • gin version (or commit ref): 1.5.0
  • operating system: windows/amd64
@linvis
Copy link
Contributor

linvis commented Mar 9, 2020

I reproduce panic, but reason is not same as yours, my panic reason is write tcp [::1]:5000->[::1]:52692: write: protocol wrong type for socket
actually is a broken pipe error, caused by client closed abnormally.

Then I use wireshark, and found why panic.

image

we have 8193 bytes to send, go http default buffer size is 4096 , so tcp send window is 4096, thus we have to send two packets, before server send, due to 2s sleep, client had disconnected tcp, so after send one packets, server receive a RST and know tcp is closed, then when send the second packet, if fail and return a error.

but in some times, if server send "faster", before receive a RST, it had already sent out all packets, there will be no panic.
image

@mrbaowei
Copy link

mrbaowei commented Jun 9, 2020

I reproduce panic, but reason is not same as yours, my panic reason is write tcp [::1]:5000->[::1]:52692: write: protocol wrong type for socket
actually is a broken pipe error, caused by client closed abnormally.

Then I use wireshark, and found why panic.

image

we have 8193 bytes to send, go http default buffer size is 4096 , so tcp send window is 4096, thus we have to send two packets, before server send, due to 2s sleep, client had disconnected tcp, so after send one packets, server receive a RST and know tcp is closed, then when send the second packet, if fail and return a error.

but in some times, if server send "faster", before receive a RST, it had already sent out all packets, there will be no panic.
image

Do you know how to solve it?

@linvis
Copy link
Contributor

linvis commented Jun 16, 2020

I reproduce panic, but reason is not same as yours, my panic reason is write tcp [::1]:5000->[::1]:52692: write: protocol wrong type for socket
actually is a broken pipe error, caused by client closed abnormally.
Then I use wireshark, and found why panic.
image
we have 8193 bytes to send, go http default buffer size is 4096 , so tcp send window is 4096, thus we have to send two packets, before server send, due to 2s sleep, client had disconnected tcp, so after send one packets, server receive a RST and know tcp is closed, then when send the second packet, if fail and return a error.
but in some times, if server send "faster", before receive a RST, it had already sent out all packets, there will be no panic.
image

Do you know how to solve it?

for the present, gin will panic this error

func (c *Context) Render(code int, r render.Render) {
	c.Status(code)

	if !bodyAllowedForStatus(code) {
		r.WriteContentType(c.Writer)
		c.Writer.WriteHeaderNow()
		return
	}

	if err := r.Render(c.Writer); err != nil {
		panic(err)
	}
}

If you want to ignore, maybe you can filter the error and ignore it.

@brookxs
Copy link

brookxs commented Jan 27, 2021

I had solve it. Don't use the default recover middleware and define a new one like this.

func RequestCancelRecover() gin.HandlerFunc {
	return func(c *gin.Context) {
		defer func() {
			if err := recover(); err != nil {
				fmt.Println("client cancel the request")
				c.Request.Context().Done()
			}
		}()
		c.Next()
	}
}

use in the code for example:

r := gin.New()
r.Use(gin.Logger(), RequestCancelRecover())
r.GET("/", controller.Index)
r.Run(":8000")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants