http2 WriteTimeout: Issue 49741 in go1.17.6 or not?

[frankre...@gmail.com]

https://github.com/golang/go/issues/49741

x/net/http2: http.Server.WriteTimeout does not fire if the http2 stream's window is out of space. #49741

The 1.17 back port issue:

x/net/http2: http.Server.WriteTimeout does not fire if the http2 stream's window is out of space. [1.17 backport] #49921

Loaded go1.17.6 on Mac and  Linux machines. Saw the release notes say issue #49741 is cherry-picked as #49921. Looked at the issue and found a very concise hanging example and expected the example would no longer hang. But it hangs on both Mac and Linux.

I admit to being confused by how the x/net/http2 source is being vendored. I did not find the source files in the go1.17.6 source tree created from the go1.17.6.tar.gz download.

Here's the example @davecheney provided:

```go

package main import ( "io" "log" "net/http" "net/http/httptest" "strings" "time" ) func handler(w http.ResponseWriter, r *http.Request) { data := strings.Repeat("x", 1<<16) tick := time.NewTicker(1 * time.Millisecond) defer tick.Stop() for { select { case <-tick.C: n, err := io.WriteString(w, data) log.Printf("wrote %d, err %v", n, err) if err != nil { return } case <-r.Context().Done(): log.Printf("context cancelled") return } } } func main() { sv := httptest.NewUnstartedServer(http.HandlerFunc(handler)) sv.EnableHTTP2 = true sv.Config.WriteTimeout = 1 * time.Second sv.StartTLS() resp, err := sv.Client().Get(sv.URL + "/") if err != nil { log.Fatal(err) } defer resp.Body.Close() select {} // block forever }

```

[Ian Lance Taylor]

On Fri, Jan 28, 2022 at 6:44 AM frankre...@gmail.com
<frankre...@gmail.com> wrote:
>
> https://github.com/golang/go/issues/49741
>
> x/net/http2: http.Server.WriteTimeout does not fire if the http2 stream's window is out of space. #49741
>
> The 1.17 back port issue:
> x/net/http2: http.Server.WriteTimeout does not fire if the http2 stream's window is out of space. [1.17 backport] #49921
>
> Loaded go1.17.6 on Mac and Linux machines. Saw the release notes say issue #49741 is cherry-picked as #49921. Looked at the issue and found a very concise hanging example and expected the example would no longer hang. But it hangs on both Mac and Linux.
>
> I admit to being confused by how the x/net/http2 source is being vendored. I did not find the source files in the go1.17.6 source tree created from the go1.17.6.tar.gz download.

The x/net/http2 sources are bundled into the generated file
net/http/h2_bundle.go.

> Here's the example @davecheney provided:

That program is always going to hang, because the main function never
returns. I'm not sure what the buggy behavior is here.

Ian

[frankre...@gmail.com]

Sorry. I misrepresented the problem. The issue was that the example should print "wrote 4096, err http2: stream closed" after a second, having filled the buffer with 64 64KiB writes first. Even with go1.17.6, it doesn't do that. The 65th write is not timing out.

But it does work if I use the recent golang.org/x/net.

I see the http2 code is in h2_bundle.go and the cherry picked changes are in fact there, in the go1.17.6 tagged checkout. So the cherry-pick operation was done correctly. Maybe that cherry-pick was necessary but it appears it isn't sufficient.

To get the intended behavior, the timeout after a second of being hung, I still have to require the golang.org/x/net from 20220127 and configure the server with it

    func main() {
        sv := httptest.NewUnstartedServer(http.HandlerFunc(handler))

        err := http2.ConfigureServer(sv.Config, &http2.Server{})

        if err != nil {
            log.Fatal(err)
        }

        // defer sv.Close()

        sv.EnableHTTP2 = true
        sv.Config.WriteTimeout = 1 * time.Second
        sv.StartTLS()

        resp, err := sv.Client().Get(sv.URL + "/")
        if err != nil {
            log.Fatal(err)
        }
        defer resp.Body.Close()

        select {} // block forever
    }

My go.mod:

    module main

    go 1.17

    require golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd

    require golang.org/x/text v0.3.7 // indirect

[frankre...@gmail.com]

I've narrowed down the difference between the case where it fails to timeout and where it does timeout to whether the http2 ConfigureServer is manually called on the *http.Server. I had to export the http2ConfigureServer in h2_bundle.go to be able to call it from the std http package but when I did, the timeout was honored. Without it, the timeout on the http2 stream doesn't happen.

I've updated the issue #49741 although it is closed so I don't know if anyone will see it.

[Antonio Ojea]

Hi,

I was surprised that this wasn't working, but it turns out that there are two "WriteScheduler"s: RandomWriteScheduler and PriorityWriteScheduler, and both have the same problem but only one was fixed.

The default WriteScheduler is different for x/net and std, that is why you observe a different behavior.

I've submitted two patches:

To fix the same bug in the PriorityWriteScheduler

https://go-review.googlesource.com/c/net/+/382014/

to use the same WriteScheduler by the default

https://go-review.googlesource.com/c/net/+/382015/1

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/e07ab828-f8a1-4fca-8aa3-4f7205d058ean%40googlegroups.com.