Io.copy hangs on hijacked connection inside Golang http server


(Waleed Gadelkareem) #1

Since the hijacked connection does not have a context. It is hard to predict if the connection is closed or not. I added a counter to measure the number of connections held by the io.copy and the results is some of the connections are on hold for a long time even after the client disconnects and the timeout is fired.

package main

import (
	"context"
	"fmt"
	"github.com/gadelkareem/go-helpers"
	"io"
	"net"
	"net/http"
	"sync"
	"sync/atomic"
	"time"
)

const Timeout = 5 * time.Second

var counter int64

func main() {
	go func() {
		for range time.Tick(Timeout) {
			fmt.Printf("%d requests being processed\n", atomic.LoadInt64(&counter))
		}
	}()

	s := &http.Server{
		Addr:    ":8888",
		Handler: http.HandlerFunc(Serve),
	}
	s.ListenAndServe()
}

func Serve(w http.ResponseWriter, r *http.Request) {

	atomic.AddInt64(&counter, 1)

	if r.Method == http.MethodConnect {
		// HTTPS
		hij, ok := w.(http.Hijacker)
		if !ok {
			return
		}

		hj, bufrw, err := hij.Hijack()
		h.PanicOnError(err)
		bufrw.WriteString("HTTP/1.0 200 OK\r\n\r\n")
		bufrw.Flush()

		target, err := net.DialTimeout("tcp", r.URL.Host, Timeout)
		h.PanicOnError(err)

		// Not sure if this is needed since we add a context timeout
		// Just leaving it for the example
		dl := time.Now().Add(Timeout)
		err = hj.SetDeadline(dl)
		h.PanicOnError(err)
		err = target.SetDeadline(dl)
		h.PanicOnError(err)

		var wg sync.WaitGroup
		ctx, cancel := context.WithTimeout(r.Context(), Timeout)
		defer cancel()
		go func() {
			for {
				select {
				case <-ctx.Done():
					hj.Close()
					target.Close()
					println("context canceled")
					return
				}
			}
		}()
		targetTCP, _ := target.(*net.TCPConn)
		hjTCP, _ := hj.(*net.TCPConn)
		wg.Add(2)
		go copyAndClose(targetTCP, hjTCP, &wg)
		go copyAndClose(hjTCP, targetTCP, &wg)
		wg.Wait()
	} else {
		//ignore HTTP for this example
	}

	atomic.AddInt64(&counter, -1)

	return
}

func copyAndClose(dst, src *net.TCPConn, wg *sync.WaitGroup) {
	io.Copy(dst, src)
	dst.CloseWrite()
	src.CloseRead()
	wg.Done()
}

You can fork it here

Is there a problem with the code or could that be a potential bug?


(Waleed Gadelkareem) #2

Never mind that was the return on non hijackable connections before the decrement of the counter