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/http: CancelRequest break another irrelevant request #9496

Closed
carter2000 opened this issue Jan 3, 2015 · 1 comment

Comments

Projects
None yet
5 participants
@carter2000
Copy link

commented Jan 3, 2015

  • net/http.persistConn.readLoop: If the response has no body, putIdleConn will be called immediately so another request can reuse the persistConn, then setReqCanceler(rc.req, nil) is called.
  • The problem is: If persistConn is reused by another-request and CancelRequest(old-request) is called before setReqCanceler(old-request, nil), the another-request will failed.
  • Source code: https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/transport.go#L920
        if alive && !hasBody {
            alive = !pc.sawEOF &&
                pc.wroteRequest() &&
                pc.t.putIdleConn(pc)     // !!!!! 'pc' can be reused after function called.
        }

        rc.ch <- responseAndError{resp, err}

        // Wait for the just-returned response body to be fully consumed
        // before we race and peek on the underlying bufio reader.
        if waitForBodyRead != nil {
            select {
            case alive = <-waitForBodyRead:
            case <-pc.closech:
                alive = false
            }
        }

        pc.t.setReqCanceler(rc.req, nil)  // !!!!! CancelRequest may be invoked before function called

        if !alive {
            pc.close()
        }
  • Reproduce, client will print "Get http://localhost:12345: net/http: transport closed before response was received"
diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go
index b1cc632..de3d15f 100644
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -860,6 +860,8 @@ func (pc *persistConn) readLoop() {
                                pc.t.putIdleConn(pc)
                }

+               time.Sleep(10e9) // to simulate an delayed schedule(for easy reproduce)
+
                rc.ch <- responseAndError{resp, err}

                // Wait for the just-returned response body to be fully consumed
// server
package main

import (
    "fmt"
    "net/http"
    "time"
)

func main() {

    var i int
    http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
        i++
        if i > 1 {
            time.Sleep(5e9)
        }
    })
    err := http.ListenAndServe(":12345", nil)
    fmt.Println("ListenAndServer", err)
}
// client
package main

import (
    "fmt"
    "net/http"
    "time"
)

func main() {

    tr := &http.Transport{}
    client := &http.Client{
        Transport: tr,
    }
    done := make(chan error, 1)
    requestCanceled, _ := http.NewRequest("GET", "http://localhost:12345", nil)
    requestIrrelevant, _ := http.NewRequest("GET", "http://localhost:12345", nil)
    go func() {
        time.Sleep(2e9) // waiting for requestToBeCanceled to be responsed
        go func() {
            time.Sleep(2e9) // waiting for requestIrrelevant to reuse the tcp connection
            tr.CancelRequest(requestCanceled)
        }()
        _, err := client.Do(requestIrrelevant)
        done <- err
    }()
    client.Do(requestCanceled)

    errIrrelevant := <-done
    if errIrrelevant != nil {
        fmt.Println(errIrrelevant)
    }
}

@bradfitz bradfitz self-assigned this Jan 3, 2015

@rsc rsc added this to the Go1.5Maybe milestone Apr 10, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 20, 2015

@bradfitz bradfitz closed this in b016eba Apr 20, 2015

@mikioh mikioh modified the milestones: Go1.5, Go1.5Maybe May 15, 2015

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

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.