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

Jump to bottom

x/net/http2: serverConn.readFrames goroutine leak #12733

Closed
bradfitz opened this issue Sep 24, 2015 · 1 comment
Closed

x/net/http2: serverConn.readFrames goroutine leak #12733

bradfitz opened this issue Sep 24, 2015 · 1 comment
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@bradfitz
Copy link
Contributor

Originally from @dvyukov at bradfitz/http2#58, @dvyukov wrote:

The following test leaks serverConn.readFrames goroutines:

package http2

import (
    "io"
    "net"
    "net/http"
    "testing"
    "time"
)

var data = "PRI * HTTP/2.0\r\n\r\nSM" +
    "\r\n\r\n\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x96\x01\x00nf" +
    "i\r\n\x00\x00\x00\t\ai\n\r\x01#-9---\n\x00" +
    "G\x01#?M\x0f\n\r\n\x00\x00\x00\t\x0f\n\n\r\x01#-" +
    "9---\n\x00G\x01#?M\x0f\n\x00G\x01#?M\x0f" +
    "\n\r\n\x00\x00\x00\t\x0f\n\n\r\x01#-9---\n\x00" +
    "G\x01#?\n\x00\x00\x00\x04\x00M\x0f\n\r\n\x00\x00\x01#-" +
    "----[\x00\x00\r\n\x00\x00\x01#-----[\x00" +
    "\x00\x01\xff\xffM\r\n\r\n\x00\x00\x01#-----\n\x00" +
    "\x00\r\n\x00\x00\x19\t\ai\n\r\n\x00\x00\x00\t\ai\n\r" +
    "\n\x00"

func TestFuzz(t *testing.T) {
    for i := 0; i < 10; i++ {
        s := &http.Server{}
        s2 := &Server{MaxReadFrameSize: 1 << 16, PermitProhibitedCipherSuites: true}
        c := &MyConn{[]byte(data), false, false}
        s2.handleConn(s, c, http.HandlerFunc(handler))
        if !c.closed {
            panic("connection is not closed")
        }
    }
    time.Sleep(time.Second)
    panic("кeady or not, here I come")
}

func handler(w http.ResponseWriter, req *http.Request) {
    w.Write([]byte("hello"))
}

type MyConn struct {
    data    []byte
    closed  bool
    written bool
}

func (c *MyConn) Read(b []byte) (n int, err error) {
    if len(c.data) == 0 {
        return 0, io.EOF
    }
    n = copy(b, c.data)
    c.data = c.data[n:]
    return
}

func (c *MyConn) Write(b []byte) (n int, err error) {
    c.written = true
    return len(b), nil
}

func (c *MyConn) Close() error {
    c.closed = true
    return nil
}

func (c *MyConn) LocalAddr() net.Addr {
    return &net.TCPAddr{net.IP{127, 0, 0, 1}, 49706, ""}
}

func (c *MyConn) RemoteAddr() net.Addr {
    return &net.TCPAddr{net.IP{127, 0, 0, 1}, 49706, ""}
}

func (c *MyConn) SetDeadline(t time.Time) error {
    return nil
}

func (c *MyConn) SetReadDeadline(t time.Time) error {
    return nil
}

func (c *MyConn) SetWriteDeadline(t time.Time) error {
    return nil
}
panic: кeady or not, here I come

goroutine 5 [running]:
testing.tRunner.func1(0xc20801a1b0)
    src/testing/testing.go:446 +0x174
github.com/bradfitz/http2.TestFuzz(0xc20801a1b0)
    src/github.com/bradfitz/http2/fuzz_test.go:34 +0x2ab
testing.tRunner(0xc20801a1b0, 0xa4c040)
    src/testing/testing.go:452 +0x9b
created by testing.RunTests
    src/testing/testing.go:560 +0xa2a

goroutine 1 [chan receive]:
testing.RunTests(0x90c700, 0xa4be60, 0x61, 0x61, 0xa4e601)
    src/testing/testing.go:561 +0xa6a
testing.(*M).Run(0xc20804ff38, 0x7f53aa89b368)
    src/testing/testing.go:490 +0x73
main.main()
    github.com/bradfitz/http2/_test/_testmain.go:250 +0x119

goroutine 9 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808c280)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 14 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808c500)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 18 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808c780)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 21 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808ca00)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 24 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808cc80)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 27 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808cf00)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 30 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808d180)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 33 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808d400)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 36 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808d680)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

goroutine 39 [chan send]:
github.com/bradfitz/http2.(*serverConn).readFrames(0xc20808d900)
    src/github.com/bradfitz/http2/server.go:544 +0x117
created by github.com/bradfitz/http2.(*serverConn).serve
    src/github.com/bradfitz/http2/server.go:626 +0x69c

on commit b6255645465a25b25f804acb9b3a54009e80c2a4
@bradfitz bradfitz self-assigned this Sep 24, 2015
@bradfitz bradfitz added this to the Go1.6 milestone Sep 24, 2015
@bradfitz bradfitz mentioned this issue Sep 24, 2015
Closed
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Oct 12, 2016
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@bradfitz
http2: fix handling of errors from the readFrames goroutine
919dd34 
There was a design problem earlier where the serve goroutine assumed
that the readFrame goroutine could return only connection-level
errors, but the readFrames goroutine (and the underlying Framer)
assumed it could return stream-level errors (type StreamError) and
have them handled as stream errors in the layers above. That's how it
should have been, and what this CL does.

Now readFrames returns both the Frame and error from ReadFrames
together as a pair, and an error isn't necessarily fatal to the
connection.

Fixes golang/go#12733
Fixes bradfitz/http2#53

Change-Id: If4406ceaa019886893d3c61e6bfce25ef74560d3
Reviewed-on: https://go-review.googlesource.com/15735
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.6
Development

No branches or pull requests
2 participants
@bradfitz @gopherbot