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

runtime: broken pipe where use http client in ServeHTTP #12931

Closed
xlvector opened this issue Oct 14, 2015 · 9 comments
Closed

runtime: broken pipe where use http client in ServeHTTP #12931

xlvector opened this issue Oct 14, 2015 · 9 comments
Milestone

Comments

@xlvector
Copy link

I use go version go1.5.1 darwin/amd64.

I write a simple http server like:

package main

import (
        "log"
        "net/http"
        "os"
        "os/signal"
)

func main() {
        c := make(chan os.Signal, 1)
        signal.Notify(c)
        go func() {
                for s := range c {
                        log.Println("recv signal: ", s.String())
                }
        }()

        http.HandleFunc("/hello", func(rw http.ResponseWriter, r *http.Request) {
                resp, _ := http.Get("http://www.baidu.com")
                if resp != nil && resp.Body != nil {
                        resp.Body.Close()
                }
                http.Error(rw, "ok", http.StatusOK)
        })
        log.Fatal(http.ListenAndServe(":8888", nil))
}

Then, If I curl http://127.0.0.1:8888/hello and then press Ctrl-C very quickly, the server will receive "broken pipe" signal. And if server receive many broken pipe signal, the process will exit.

@davecheney
Copy link
Contributor

How are you running this program? Are you using go run ?

On Wed, 14 Oct 2015, 23:46 Xiang Liang notifications@github.com wrote:

I use go version go1.5.1 darwin/amd64.

I write a simple http server like:

package main

import (
"log"
"net/http"
"os"
"os/signal"
)

func main() {
c := make(chan os.Signal, 1)
signal.Notify(c)
go func() {
for s := range c {
log.Println("recv signal: ", s.String())
}
}()

http.HandleFunc("/hello", func(rw http.ResponseWriter, r *http.Request) {
        resp, _ := http.Get("http://www.baidu.com")
        if resp != nil && resp.Body != nil {
                resp.Body.Close()
        }
        http.Error(rw, "ok", http.StatusOK)
})
log.Fatal(http.ListenAndServe(":8888", nil))

}

Then, If I curl http://127.0.0.1:8888/hello and then press Ctrl-C very
quickly, the server will receive "broken pipe" signal. And if server
receive many broken pipe signal, the process will exit.


Reply to this email directly or view it on GitHub
#12931.

@xlvector
Copy link
Author

I go build it and then run. Does not use go run.

@davecheney
Copy link
Contributor

Thank you for confirming.

On Thu, Oct 15, 2015 at 12:05 AM, Xiang Liang notifications@github.com
wrote:

I go build it and then run. Does not use go run.


Reply to this email directly or view it on GitHub
#12931 (comment).

@ianlancetaylor ianlancetaylor changed the title Broken pipe where use http client in ServeHTTP runtime: Broken pipe where use http client in ServeHTTP Oct 14, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Oct 14, 2015
@xlvector
Copy link
Author

Following code will help you reproduce this problem more easily:

package main

import (
    "log"
    "math/rand"
    "net/http"
    "os"
    "os/signal"
    "time"
)

func init() {
    rand.Seed(time.Now().UnixNano())
}

var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")

func RandStringRunes(n int) string {
    b := make([]rune, n)
    for i := range b {
        b[i] = letterRunes[rand.Intn(len(letterRunes))]
    }
    return string(b)
}

func main() {
    c := make(chan os.Signal, 1)
    signal.Notify(c)
    go func() {
        for s := range c {
            log.Println("recv signal: ", s.String())
        }
    }()

    http.HandleFunc("/hello", func(rw http.ResponseWriter, r *http.Request) {
        c := http.Client{
            Transport: &http.Transport{
                DisableKeepAlives: true,
            },
        }
        req, _ := http.NewRequest("GET", "http://www.baidu.com", nil)
        resp, _ := c.Do(req)
        if resp != nil && resp.Body != nil {
            resp.Body.Close()
        }
        http.Error(rw, RandStringRunes(10000000), http.StatusOK)
    })
    log.Fatal(http.ListenAndServe(":8888", nil))
}

@xlvector xlvector reopened this Oct 14, 2015
@rsc
Copy link
Contributor

rsc commented Nov 5, 2015

It's correct for the signal loop to report getting SIGPIPE, because the process is getting SIGPIPEs (writing to closed connections). That's a fact of Unix, with nothing we can do about that.

If the process can be made to exit, it means that something wrote 10 times in a row to a specific closed network connection, ignoring the error. That causes the runtime to turn off its SIGPIPE handler and raise SIGPIPE itself. If an HTTP server can be forced to do that, either the SIGPIPE heuristic is buggy or the HTTP code is.

FWIW, #11845 wants more SIGPIPE; this issue wants less SIGPIPE.

@rsc rsc changed the title runtime: Broken pipe where use http client in ServeHTTP runtime: broken pipe where use http client in ServeHTTP Nov 5, 2015
@ianlancetaylor
Copy link
Contributor

I don't necessarily want more SIGPIPE in #11845, although that was my initial suggestion. What I really want is to get away from the magic number 10. I think we should either ignore SIGPIPE or raise SIGPIPE immediately. Since different programs have different needs, I think we may need to have a function that sets the desired behaviour. For an HTTP server, indeed for any program that calls net.Listen, I think the correct default to ignore SIGPIPE. For a command line program that is not a server, I think the correct default is to die on SIGPIPE.

@xlvector
Copy link
Author

Yes, you are write. For other signals, we can ignore them. But I do not know why we need magic number 10 for SIGPIPE.

@ianlancetaylor
Copy link
Contributor

@xlvector there is a proposal in #11845.

@rsc
Copy link
Contributor

rsc commented Nov 23, 2015

Closing as duplicate of #11845. That work will fix this problem.

@rsc rsc closed this as completed Nov 23, 2015
@golang golang locked and limited conversation to collaborators Nov 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants