From 83a5226e1a36d33d977f25192647011fb8206d33 Mon Sep 17 00:00:00 2001
From: Laurence Withers <l@lwithers.me.uk>
Date: Wed, 6 Jul 2022 09:51:25 +0100
Subject: [PATCH] handler: drop sendfile(2) support
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After some experimentation, I found that the sendfile(2) support did not
really save any time compared to just write(2) from an already
memory-mapped file.

After some reading, I think open/sendfile is supposed to be slightly
more efficient than open/mmap/write — but if we already did the mmap
step, then it doesn't save us much.

Moreover, the code to support sendfile(2) is a bit icky, and also forces
us to close the HTTP connection after serving a file.
---
 handler.go | 89 +-----------------------------------------------------
 1 file changed, 1 insertion(+), 88 deletions(-)

diff --git a/handler.go b/handler.go
index 92da9d4..6c34f2f 100644
--- a/handler.go
+++ b/handler.go
@@ -2,14 +2,12 @@ package htpack
 
 import (
 	"fmt"
-	"net"
 	"net/http"
 	"os"
 	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
-	"syscall"
 	"time"
 
 	"golang.org/x/sys/unix"
@@ -21,14 +19,13 @@ const (
 	encodingBrotli = "br"
 )
 
-// TODO: logging
-
 // New returns a new handler. Standard security headers are set.
 func New(packfile string) (*Handler, error) {
 	f, err := os.Open(packfile)
 	if err != nil {
 		return nil, err
 	}
+	defer f.Close()
 
 	fi, err := f.Stat()
 	if err != nil {
@@ -37,19 +34,16 @@ func New(packfile string) (*Handler, error) {
 	mapped, err := unix.Mmap(int(f.Fd()), 0, int(fi.Size()),
 		unix.PROT_READ, unix.MAP_SHARED)
 	if err != nil {
-		f.Close()
 		return nil, err
 	}
 
 	_, dir, err := packed.Load(f)
 	if err != nil {
 		unix.Munmap(mapped)
-		f.Close()
 		return nil, err
 	}
 
 	h := &Handler{
-		f:         f,
 		mapped:    mapped,
 		dir:       dir.Files,
 		headers:   make(map[string]string),
@@ -67,7 +61,6 @@ func New(packfile string) (*Handler, error) {
 
 // Handler implements http.Handler and allows options to be set.
 type Handler struct {
-	f         *os.File
 	mapped    []byte
 	dir       map[string]*packed.File
 	headers   map[string]string
@@ -203,87 +196,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	if req.Method == "HEAD" {
 		return
 	}
-	h.sendfile(w, data, offset, length)
-}
 
-func (h *Handler) sendfile(w http.ResponseWriter, data *packed.FileData,
-	offset, length uint64,
-) {
-	hj, ok := w.(http.Hijacker)
-	if !ok {
-		// fallback
-		h.copyfile(w, data, offset, length)
-		return
-	}
-
-	conn, buf, err := hj.Hijack()
-	if err != nil {
-		// fallback
-		h.copyfile(w, data, offset, length)
-		return
-	}
-
-	tcp, ok := conn.(*net.TCPConn)
-	if !ok {
-		// fallback
-		h.copyfile(w, data, offset, length)
-		return
-	}
-	defer tcp.Close()
-
-	rawsock, err := tcp.SyscallConn()
-	if err == nil {
-		err = buf.Flush()
-	}
-	if err != nil {
-		// error only returned if the underlying connection is broken,
-		// so there's no point calling sendfile
-		return
-	}
-
-	var breakErr error
-	off := int64(data.Offset + offset)
-	remain := length
-
-	for breakErr == nil && remain > 0 {
-		// sendfile(2) can send a maximum of 1GiB
-		var amt int
-		if remain > (1 << 30) {
-			amt = (1 << 30)
-		} else {
-			amt = int(remain)
-		}
-
-		// behaviour of control function:
-		//  · some bytes written: sets written > 0, returns true (breaks
-		//                        out of loop on first write)
-		//  · EAGAIN: returns false (causes Write() to loop until
-		//            success or permanent failure)
-		//  · other error: sets breakErr
-		var written int
-		rawsock.Write(func(outfd uintptr) bool {
-			written, err = unix.Sendfile(int(outfd), int(h.f.Fd()), &off, amt)
-			switch err {
-			case nil:
-				return true
-			case syscall.EAGAIN:
-				return false
-			default:
-				breakErr = err
-				return true
-			}
-		})
-
-		// we may have had a partial write, or file may have been > 1GiB
-		remain -= uint64(written)
-	}
-}
-
-// copyfile is a fallback handler that uses write(2) on our memory-mapped data
-// to push out the response.
-func (h *Handler) copyfile(w http.ResponseWriter, data *packed.FileData,
-	offset, length uint64,
-) {
 	offset += data.Offset
 	w.Write(h.mapped[offset : offset+length])
 }