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

net: ignore WSAECONNRESET AcceptEx error #6987

Closed
gopherbot opened this issue Dec 19, 2013 · 17 comments

Comments

Projects
None yet
5 participants
@gopherbot
Copy link

commented Dec 19, 2013

by trivital:

run simplest server code below (under windows 7 64bit, with go 1.2)

package main

import (
    "fmt"
    "net/http"
)

func rootHandler(resp http.ResponseWriter, req *http.Request) {
    fmt.Fprintf(resp, "haha")
}

func main() {
    http.HandleFunc("/x", rootHandler)
    err := http.ListenAndServe(":8888", nil)

    panic(err.Error())
}


if i ab load testing this piece of server for a long period, i randomly get this err:

panic: AcceptEx tcp [::]:8888: An existing connection was forcibly closed by the remote
host.

goroutine 1 [running]:
runtime.panic(0x5aabe0, 0xc084872a20)
    C:/Users/ADMINI~1/AppData/Local/Temp/2/bindist667667715/go/src/pkg/runtime/panic.c:266 +0xc8
main.main()
    E:/Tools/liteide/bin/src/testsvr/mysvr.go:16 +0xb5


same code on debian 64bit, no error no matter how
@dvyukov

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

Comment 1:

It seems that we need to ignore WSAECONNRESET from WSAAccept. And maybe also
WSAECONNREFUSED. Why does windows even return them?..
@alexbrainman

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

Comment 2:

Do you propose to ignore them in net/http or in net? We shouldn't be "ignoring" OS
errors (if possible). It should be up to the user what to do with them.
Alex
@gopherbot

This comment has been minimized.

Copy link
Author

commented Dec 20, 2013

Comment 3 by trivital:

Alex, the problem is that you need to write different code for different OSes.
Plus, I cant handle this error even if I wanted to. http.ListenAndServe just plain
crashed.
@dvyukov

This comment has been minimized.

Copy link
Member

commented Dec 20, 2013

Comment 4:

I would say that this is not an OS error, it's more of an informational notification
exposed through standard error reporting channel. Think of EINTR.
@gopherbot

This comment has been minimized.

Copy link
Author

commented Dec 20, 2013

Comment 5 by trivital:

net/http snippet 
func (srv *Server) Serve(l net.Listener) error {
    defer l.Close()
    var tempDelay time.Duration // how long to sleep on accept failure
    for {
        rw, e := l.Accept() //this is where it happens ...
        if e != nil {
            if ne, ok := e.(net.Error); ok && ne.Temporary() {
                if tempDelay == 0 {
                    tempDelay = 5 * time.Millisecond
                } else {
                    tempDelay *= 2
                }
                if max := 1 * time.Second; tempDelay > max {
                    tempDelay = max
                }
                log.Printf("http: Accept error: %v; retrying in %v", e, tempDelay)
                time.Sleep(tempDelay)
                continue
            }
            return e
        }
        tempDelay = 0
        c, err := srv.newConn(rw)
        if err != nil {
            continue
        }
        go c.serve()
    }
}
@gopherbot

This comment has been minimized.

Copy link
Author

commented Dec 20, 2013

Comment 6 by trivital:

i think we should continue on e.(net.Error)==WSAECONNRESET instead of return e
@alexbrainman

This comment has been minimized.

Copy link
Member

commented Dec 20, 2013

Comment 7:

WSAECONNRESET is actually about new connection, not about listening socket. I suspect,
it just the way AcceptEx is implemented that it returns that error. I suppose, we should
ignore it.
Alex

Labels changed: added release-go1.3, repo-main, os-windows.

Owner changed to @alexbrainman.

Status changed to Accepted.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Dec 20, 2013

Comment 8 by trivital:

exactly Alex, agree!
@dvyukov

This comment has been minimized.

Copy link
Member

commented Dec 20, 2013

Comment 9:

Labels changed: added release-go1.2.1, removed release-go1.3.

@alberts

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2014

Comment 10:

This is related to issue #6163, which is about Accept not ignoring all temporary errors
on Linux.
@alexbrainman

This comment has been minimized.

Copy link
Member

commented Jan 9, 2014

Comment 11:

Please review https://golang.org/cl/49490043/
Alex

Status changed to Started.

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Jan 12, 2014

Comment 12:

This issue was closed by revision c7ef348.

Status changed to Fixed.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Jan 30, 2014

Comment 13 by rivercheng:

It seems this issue still exists on go1.2 amd 64.
@alexbrainman

This comment has been minimized.

Copy link
Member

commented Jan 30, 2014

Comment 14:

@rivercheng,
You won't see this fix as part of go1.2 - it has been fixed after go1.2 has been
released.
This is issue is marked as "Release-Go1.2.1", so, I expect, this change should be part
of go-1.2.1.
Alex
@gopherbot

This comment has been minimized.

Copy link
Author

commented Feb 3, 2014

Comment 15 by rivercheng:

Thanks, Alex.
I forgot to check the date and thought this was a bug fixed long ago. :)
@rsc

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2014

Comment 16:

This issue was closed by revision 4e312b5239f0.

@rsc rsc added this to the Go1.2.1 milestone Apr 14, 2015

@rsc rsc removed the release-go1.2.1 label Apr 14, 2015

rsc added a commit that referenced this issue May 11, 2015

[release-branch.go1.2] net: ignore some errors in windows Accept
««« CL 49490043 / 7ecbc2b8ec97
net: ignore some errors in windows Accept

Fixes #6987

R=golang-codereviews, dvyukov
CC=golang-codereviews
https://golang.org/cl/49490043
»»»

LGTM=r
R=golang-codereviews, r
CC=golang-dev
https://golang.org/cl/69780044
@gopherbot

This comment has been minimized.

Copy link
Author

commented May 12, 2015

CL https://golang.org/cl/9946 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 25, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.