hashicorp/yamux

http.Serve(Session, nil) hangs on abortPendingRead when using Hijack()

hagna opened this issue · 2 comments

hagna commented

This minimal example hangs on go 1.10 when I add log.Printf to hijackLocked() (net/http/server.go:300). It hangs on abortPendingRead() (net/http/server.go:692) when I can get it to hang. What are some possible issues with hijacking?

package main

import (
	"context"
	"flag"
	"fmt"
	"github.com/hashicorp/yamux"
	"net"
	"net/http"
	"time"
)

var endpoint = flag.String("ep", "127.0.0.1:8000", "endpoint")

func client() {
	// Get a TCP connection
	conn, err := net.Dial("tcp", *endpoint)
	if err != nil {
		panic(err)
	}

	// Setup client side of yamux
	session, err := yamux.Client(conn, nil)
	if err != nil {
		panic(err)
	}
	http.Serve(session, nil)

}

func server() {
	// Accept a TCP connection
	listener, err := net.Listen("tcp", *endpoint)
	if err != nil {
		panic(err)
	}
	conn, err := listener.Accept()
	if err != nil {
		panic(err)
	}

	// Setup server side of yamux
	session, err := yamux.Server(conn, nil)
	if err != nil {
		panic(err)
	}
	tr := &http.Transport{
		DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
			return session.Open()
		},
	}
	client := &http.Client{Transport: tr}
	resp, err := client.Get("http://127.0.0.1:8000/works")
	if err != nil {
		panic(err)
	}
	fmt.Println(resp)
	fmt.Println("hanging in abortPendingRead()...")
	resp, err = client.Get("http://127.0.0.1:8000/bug")
	if err != nil {
		fmt.Println("we should not see this because it should hang in abortPendingRead()")
		fmt.Println(err)
	}
	fmt.Println(resp)

}

func main() {
	flag.Parse()
	http.HandleFunc("/works", func(w http.ResponseWriter, r *http.Request) {
		fmt.Fprintf(w, "this works")
	})
	http.HandleFunc("/bug", func(w http.ResponseWriter, r *http.Request) {
		h, ok := w.(http.Hijacker)
		if !ok {
			fmt.Printf("does not support hijack")
		}
		c, _, err := h.Hijack()
		if err != nil {
			panic(err)
		}
		fmt.Fprintf(c, "this works")
		c.Close()
	})

	go server()
	time.Sleep(90 * time.Millisecond)
	client()
}

Likely related: #51

hagna commented

Thanks. Commit c435ed1 does appear to fix it, and by that I mean this:

@@ -440,12 +440,14 @@ func (s *Stream) SetDeadline(t time.Time) error {
 // SetReadDeadline sets the deadline for future Read calls.
 func (s *Stream) SetReadDeadline(t time.Time) error {
        s.readDeadline.Store(t)
+       asyncNotify(s.recvNotifyCh)
        return nil
 }
 
 // SetWriteDeadline sets the deadline for future Write calls
 func (s *Stream) SetWriteDeadline(t time.Time) error {
        s.writeDeadline.Store(t)
+       asyncNotify(s.sendNotifyCh)
        return nil
 }