From 02f40b4d05fde72d17dc8e9804eeafcf4709917b Mon Sep 17 00:00:00 2001 From: Richard Lavoie Date: Thu, 6 Aug 2026 10:57:58 -0400 Subject: [PATCH] Windows so_reuseport works with SO_REUSEADDR --- .gitignore | 2 + cmd/svcinit/BUILD.bazel | 13 ++- cmd/svcinit/main.go | 98 ++++++++++++------- cmd/svcinit/reserve_reusable_port_test.go | 70 +++++++++++++ cmd/svcinit/reserve_reusable_port_unix.go | 56 +++++++++++ cmd/svcinit/reserve_reusable_port_windows.go | 67 +++++++++++++ ...et_sockopts_for_port_assignment_windows.go | 10 +- docs/itest.md | 9 +- private/itest.bzl | 12 ++- runner/runner.go | 3 + tests/go_service/BUILD.bazel | 3 + tests/go_service/main.go | 6 +- tests/go_service/serve_windows.go | 37 ++++++- tests/so_reuseport/BUILD.bazel | 9 -- 14 files changed, 340 insertions(+), 55 deletions(-) create mode 100644 cmd/svcinit/reserve_reusable_port_test.go create mode 100644 cmd/svcinit/reserve_reusable_port_unix.go create mode 100644 cmd/svcinit/reserve_reusable_port_windows.go diff --git a/.gitignore b/.gitignore index ecb9123..030e770 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ +.ijwb/ + bazel-* MODULE.bazel.lock examples/MODULE.bazel.lock diff --git a/cmd/svcinit/BUILD.bazel b/cmd/svcinit/BUILD.bazel index 08736c7..7b4a22a 100644 --- a/cmd/svcinit/BUILD.bazel +++ b/cmd/svcinit/BUILD.bazel @@ -1,9 +1,11 @@ -load("@rules_go//go:def.bzl", "go_binary", "go_library") +load("@rules_go//go:def.bzl", "go_binary", "go_library", "go_test") go_library( name = "svcinit_lib", srcs = [ "main.go", + "reserve_reusable_port_unix.go", + "reserve_reusable_port_windows.go", "set_sockopts_for_port_assignment_unix.go", "set_sockopts_for_port_assignment_windows.go", ], @@ -49,10 +51,19 @@ go_library( "@rules_go//go/platform:solaris": [ "@org_golang_x_sys//unix", ], + "@rules_go//go/platform:windows": [ + "@org_golang_x_sys//windows", + ], "//conditions:default": [], }), ) +go_test( + name = "svcinit_test", + srcs = ["reserve_reusable_port_test.go"], + embed = [":svcinit_lib"], +) + go_binary( name = "svcinit", data = ["//cmd/get_assigned_port"], diff --git a/cmd/svcinit/main.go b/cmd/svcinit/main.go index e0c2703..6114f6c 100644 --- a/cmd/svcinit/main.go +++ b/cmd/svcinit/main.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "log" "maps" "math" @@ -124,8 +125,9 @@ func main() { listener, err := net.Listen("tcp", "127.0.0.1:0") must(err) - ports, err := assignPorts(unversionedSpecs) + ports, reservedPorts, err := assignPorts(unversionedSpecs) must(err) + defer closeReservedPorts(reservedPorts) svcctlPort := listener.Addr().(*net.TCPAddr).Port svcctlPortStr := strconv.Itoa(svcctlPort) @@ -374,9 +376,10 @@ func readServiceSpecs( func assignPorts( serviceSpecs map[string]svclib.ServiceSpec, ) ( - svclib.Ports, error, + svclib.Ports, map[string][]io.Closer, error, ) { - var toClose []net.Listener + var toClose []io.Closer + reservedPorts := map[string][]io.Closer{} ports := svclib.Ports{} for label, spec := range serviceSpecs { @@ -386,34 +389,49 @@ func assignPorts( } // Note, this can cause collisions. So be careful! - // To avoid port collisions, set the `so_reuseport_aware` option on the service definition - // and use the SO_REUSEPORT socket option in your services. + // To avoid port collisions, set so_reuseport_aware on the service definition + // and use SO_REUSEPORT on Unix or SO_REUSEADDR on Windows in your services. for portName, port := range namedPorts { - // We do a bit of a dance here to set SO_LINGER to 0. For details, see - // https://stackoverflow.com/questions/71975992/what-really-is-the-linger-time-that-can-be-set-with-so-linger-on-sockets - lc := net.ListenConfig{ - Control: func(network, address string, conn syscall.RawConn) error { - var setSockoptErr error - err := conn.Control(func(fd uintptr) { - setSockoptErr = setSockoptsForPortAssignment(fd, &syscall.Linger{ - Onoff: 1, - Linger: 0, + var reservedPort io.Closer + var err error + if spec.SoReuseportAware { + requestedPort, parseErr := strconv.Atoi(port) + if parseErr != nil || requestedPort < 0 || requestedPort > 65535 { + return nil, nil, fmt.Errorf("invalid port %q for %s", port, label) + } + reservedPort, port, err = reserveReusablePort(requestedPort) + if err != nil { + return nil, nil, err + } + } else { + // We do a bit of a dance here to set SO_LINGER to 0. For details, see + // https://stackoverflow.com/questions/71975992/what-really-is-the-linger-time-that-can-be-set-with-so-linger-on-sockets + lc := net.ListenConfig{ + Control: func(network, address string, conn syscall.RawConn) error { + var setSockoptErr error + err := conn.Control(func(fd uintptr) { + setSockoptErr = setSockoptsForPortAssignment(fd, &syscall.Linger{ + Onoff: 1, + Linger: 0, + }) }) - }) - if err != nil { - return err - } - return setSockoptErr - }, - } + if err != nil { + return err + } + return setSockoptErr + }, + } - listener, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:"+port) - if err != nil { - return nil, err - } - _, port, err = net.SplitHostPort(listener.Addr().String()) - if err != nil { - return nil, err + listener, listenErr := lc.Listen(context.Background(), "tcp", "127.0.0.1:"+port) + if listenErr != nil { + return nil, nil, listenErr + } + _, port, err = net.SplitHostPort(listener.Addr().String()) + if err != nil { + listener.Close() + return nil, nil, err + } + reservedPort = listener } qualifiedPortName := label @@ -442,15 +460,17 @@ func assignPorts( } if !spec.SoReuseportAware { - toClose = append(toClose, listener) + toClose = append(toClose, reservedPort) + } else { + reservedPorts[label] = append(reservedPorts[label], reservedPort) } } } - for _, listener := range toClose { - err := listener.Close() + for _, reservedPort := range toClose { + err := reservedPort.Close() if err != nil { - return nil, err + return nil, nil, err } } @@ -481,10 +501,20 @@ func assignPorts( serializedPorts, err := ports.Marshal() if err != nil { - return nil, err + return nil, nil, err } os.Setenv("ASSIGNED_PORTS", string(serializedPorts)) - return ports, nil + return ports, reservedPorts, nil +} + +func closeReservedPorts(reservedPorts map[string][]io.Closer) { + for label, ports := range reservedPorts { + for _, port := range ports { + if err := port.Close(); err != nil { + log.Printf("failed to close reusable port reservation for %s: %v\n", label, err) + } + } + } } func augmentServiceSpecs( diff --git a/cmd/svcinit/reserve_reusable_port_test.go b/cmd/svcinit/reserve_reusable_port_test.go new file mode 100644 index 0000000..38ee774 --- /dev/null +++ b/cmd/svcinit/reserve_reusable_port_test.go @@ -0,0 +1,70 @@ +package main + +import ( + "context" + "net" + "syscall" + "testing" + "time" +) + +func TestReusablePortReservation(t *testing.T) { + reservation, port, err := reserveReusablePort(0) + if err != nil { + t.Fatal(err) + } + defer reservation.Close() + + unawareListener, err := net.Listen("tcp4", "127.0.0.1:"+port) + if err == nil { + unawareListener.Close() + t.Fatal("listener without a reusable-port option unexpectedly claimed the reserved port") + } + + lc := net.ListenConfig{ + Control: func(network, address string, conn syscall.RawConn) error { + var setSockoptErr error + err := conn.Control(func(fd uintptr) { + setSockoptErr = setSockoptsForPortAssignment(fd, &syscall.Linger{ + Onoff: 1, + Linger: 0, + }) + }) + if err != nil { + return err + } + return setSockoptErr + }, + } + listener, err := lc.Listen(context.Background(), "tcp4", "127.0.0.1:"+port) + if err != nil { + t.Fatalf("listen on reserved port: %v", err) + } + defer listener.Close() + + acceptErr := make(chan error, 1) + go func() { + conn, err := listener.Accept() + if err != nil { + acceptErr <- err + return + } + conn.Close() + acceptErr <- nil + }() + + conn, err := net.DialTimeout("tcp4", "127.0.0.1:"+port, time.Second) + if err != nil { + t.Fatalf("dial service listener: %v", err) + } + conn.Close() + + select { + case err := <-acceptErr: + if err != nil { + t.Fatalf("accept from service listener: %v", err) + } + case <-time.After(time.Second): + t.Fatal("connection was not accepted by the service listener") + } +} diff --git a/cmd/svcinit/reserve_reusable_port_unix.go b/cmd/svcinit/reserve_reusable_port_unix.go new file mode 100644 index 0000000..a8019f6 --- /dev/null +++ b/cmd/svcinit/reserve_reusable_port_unix.go @@ -0,0 +1,56 @@ +//go:build unix + +package main + +import ( + "fmt" + "io" + "os" + "strconv" + "syscall" +) + +func reserveReusablePort(port int) (io.Closer, string, error) { + fd, err := syscall.Socket(syscall.AF_INET, syscall.SOCK_STREAM, syscall.IPPROTO_TCP) + if err != nil { + return nil, "", fmt.Errorf("socket: %w", err) + } + syscall.CloseOnExec(fd) + + file := os.NewFile(uintptr(fd), "rules_itest_reserved_reuseport") + success := false + defer func() { + if !success { + file.Close() + } + }() + + // Do not set SO_REUSEADDR here. Go TCP listeners enable it by default on + // Linux, where it would allow an unaware listener to claim a bind-only + // reservation. SO_REUSEPORT alone allows the aware service to share it. + if err := setSockoptsForPortAssignment(uintptr(fd), &syscall.Linger{ + Onoff: 1, + Linger: 0, + }); err != nil { + return nil, "", fmt.Errorf("set reusable reservation socket options: %w", err) + } + + if err := syscall.Bind(fd, &syscall.SockaddrInet4{ + Port: port, + Addr: [4]byte{127, 0, 0, 1}, + }); err != nil { + return nil, "", fmt.Errorf("bind reusable reservation socket: %w", err) + } + + addr, err := syscall.Getsockname(fd) + if err != nil { + return nil, "", fmt.Errorf("getsockname reusable reservation socket: %w", err) + } + tcpAddr, ok := addr.(*syscall.SockaddrInet4) + if !ok { + return nil, "", fmt.Errorf("getsockname returned %T, expected *syscall.SockaddrInet4", addr) + } + + success = true + return file, strconv.Itoa(tcpAddr.Port), nil +} diff --git a/cmd/svcinit/reserve_reusable_port_windows.go b/cmd/svcinit/reserve_reusable_port_windows.go new file mode 100644 index 0000000..4671787 --- /dev/null +++ b/cmd/svcinit/reserve_reusable_port_windows.go @@ -0,0 +1,67 @@ +//go:build windows + +package main + +import ( + "fmt" + "io" + "strconv" + "syscall" + + "golang.org/x/sys/windows" +) + +type windowsPortReservation struct { + socket windows.Handle +} + +func (r *windowsPortReservation) Close() error { + return windows.Closesocket(r.socket) +} + +func reserveReusablePort(port int) (io.Closer, string, error) { + socket, err := windows.WSASocket( + windows.AF_INET, + windows.SOCK_STREAM, + windows.IPPROTO_TCP, + nil, + 0, + windows.WSA_FLAG_OVERLAPPED|windows.WSA_FLAG_NO_HANDLE_INHERIT, + ) + if err != nil { + return nil, "", fmt.Errorf("socket: %w", err) + } + + success := false + defer func() { + if !success { + windows.Closesocket(socket) + } + }() + + if err := setSockoptsForPortAssignment(uintptr(socket), &syscall.Linger{ + Onoff: 1, + Linger: 0, + }); err != nil { + return nil, "", fmt.Errorf("set reusable reservation socket options: %w", err) + } + + if err := windows.Bind(socket, &windows.SockaddrInet4{ + Port: port, + Addr: [4]byte{127, 0, 0, 1}, + }); err != nil { + return nil, "", fmt.Errorf("bind reusable reservation socket: %w", err) + } + + addr, err := windows.Getsockname(socket) + if err != nil { + return nil, "", fmt.Errorf("getsockname reusable reservation socket: %w", err) + } + tcpAddr, ok := addr.(*windows.SockaddrInet4) + if !ok { + return nil, "", fmt.Errorf("getsockname returned %T, expected *windows.SockaddrInet4", addr) + } + + success = true + return &windowsPortReservation{socket: socket}, strconv.Itoa(tcpAddr.Port), nil +} diff --git a/cmd/svcinit/set_sockopts_for_port_assignment_windows.go b/cmd/svcinit/set_sockopts_for_port_assignment_windows.go index 736021d..9837520 100644 --- a/cmd/svcinit/set_sockopts_for_port_assignment_windows.go +++ b/cmd/svcinit/set_sockopts_for_port_assignment_windows.go @@ -5,6 +5,12 @@ package main import "syscall" func setSockoptsForPortAssignment(fd uintptr, l *syscall.Linger) error { - // Windows (even WSL) does not seem to support SO_REUSEPORT - return syscall.SetsockoptLinger(syscall.Handle(fd), syscall.SOL_SOCKET, syscall.SO_LINGER, l) + err := syscall.SetsockoptLinger(syscall.Handle(fd), syscall.SOL_SOCKET, syscall.SO_LINGER, l) + if err != nil { + return err + } + + // Windows has no SO_REUSEPORT. SO_REUSEADDR allows the service listener to + // bind while svcinit holds a bind-only reservation socket. + return syscall.SetsockoptInt(syscall.Handle(fd), syscall.SOL_SOCKET, syscall.SO_REUSEADDR, 1) } diff --git a/docs/itest.md b/docs/itest.md index fc76dce..fc2e850 100644 --- a/docs/itest.md +++ b/docs/itest.md @@ -19,6 +19,13 @@ query:enable-reload --@rules_itest//:enable_per_service_reload In addition, if the `hot_reloadable` attribute is set on an `itest_service`, the service manager will forward the ibazel hot-reload notification over stdin instead of restarting the service. +# Reusable port reservations + +For each service with `so_reuseport_aware = True`, the service manager adds +`RULES_ITEST_ENABLE_SO_REUSEPORT=1` to that service's environment. Services can use this signal to +enable the socket option required to share their bind-only port reservation: `SO_REUSEPORT` on Unix +or `SO_REUSEADDR` on Windows. The option must be set before binding the service socket. + # Service control The service manager exposes a HTTP server on `http://127.0.0.1:{SVCCTL_PORT}`. It can be used to @@ -75,7 +82,7 @@ All [common binary attributes](https://bazel.build/reference/be/common-definitio | port | Internal. | Label | optional | `None` | | shutdown_signal | The signal to send to the service when it needs to be shut down. Valid values are: SIGTERM and SIGKILL. SIGTERM is necessary to have proper coverage of services which needs to be gracefully terminated | String | optional | `"SIGTERM"` | | shutdown_timeout | The duration to wait by default after sending the shutdown signal before forcefully killing the service. The syntax is based on common time duration with a number, followed by the time unit. For example, `200ms`, `1s`, `2m`, `3h`, `4d`. If not defined, the value of `_default_shutdown_timeout` will be used. | String | optional | `""` | -| so_reuseport_aware | If set, the service manager will not release the autoassigned port. The service binary must use SO_REUSEPORT when binding it. This reduces the possibility of port collisions when running many service_tests in parallel, or when code binds port 0 without being aware of the port assignment mechanism.

Must only be set when `autoassign_port` is enabled or `named_ports` are used. | Boolean | optional | `False` | +| so_reuseport_aware | If set, the service manager keeps a bind-only reservation for the autoassigned port for the service manager's lifetime. The service binary must use SO_REUSEPORT on Unix or SO_REUSEADDR on Windows when binding it. This reduces the possibility of port collisions when running many service_tests in parallel, or when code binds port 0 without being aware of the port assignment mechanism.

Must only be set when `autoassign_port` is enabled or `named_ports` are used. | Boolean | optional | `False` | diff --git a/private/itest.bzl b/private/itest.bzl index 32712b0..2ea1e38 100644 --- a/private/itest.bzl +++ b/private/itest.bzl @@ -18,6 +18,13 @@ query:enable-reload --@rules_itest//:enable_per_service_reload In addition, if the `hot_reloadable` attribute is set on an `itest_service`, the service manager will forward the ibazel hot-reload notification over stdin instead of restarting the service. +# Reusable port reservations + +For each service with `so_reuseport_aware = True`, the service manager adds +`RULES_ITEST_ENABLE_SO_REUSEPORT=1` to that service's environment. Services can use this signal to +enable the socket option required to share their bind-only port reservation: `SO_REUSEPORT` on Unix +or `SO_REUSEADDR` on Windows. The option must be set before binding the service socket. + # Service control The service manager exposes a HTTP server on `http://127.0.0.1:{SVCCTL_PORT}`. It can be used to @@ -262,8 +269,9 @@ _itest_service_attrs = _itest_binary_attrs | { Named ports are accessible through the service-port mapping. For more details, see `autoassign_port`.""", ), "so_reuseport_aware": attr.bool( - doc = """If set, the service manager will not release the autoassigned port. The service binary must use SO_REUSEPORT when binding it. - This reduces the possibility of port collisions when running many service_tests in parallel, or when code binds port 0 without being + doc = """If set, the service manager keeps a bind-only reservation for the autoassigned port for the service manager's lifetime. + The service binary must use SO_REUSEPORT on Unix or SO_REUSEADDR on Windows when binding it. This reduces the possibility of port + collisions when running many service_tests in parallel, or when code binds port 0 without being aware of the port assignment mechanism. Must only be set when `autoassign_port` is enabled or `named_ports` are used.""", diff --git a/runner/runner.go b/runner/runner.go index d9f57c7..daa85f9 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -265,6 +265,9 @@ func initializeServiceCmd(ctx context.Context, instance *ServiceInstance) error for k, v := range s.Env { cmd.Env = append(cmd.Env, k+"="+v) } + if s.SoReuseportAware { + cmd.Env = append(cmd.Env, "RULES_ITEST_ENABLE_SO_REUSEPORT=1") + } cmd.Stdout = logger.New(s.Label+"> ", s.Color, os.Stdout) cmd.Stderr = logger.New(s.Label+"> ", s.Color, os.Stderr) diff --git a/tests/go_service/BUILD.bazel b/tests/go_service/BUILD.bazel index 6097cb8..c172559 100644 --- a/tests/go_service/BUILD.bazel +++ b/tests/go_service/BUILD.bazel @@ -45,6 +45,9 @@ go_library( "@rules_go//go/platform:solaris": [ "@org_golang_x_sys//unix", ], + "@rules_go//go/platform:windows": [ + "@org_golang_x_sys//windows", + ], "//conditions:default": [], }), ) diff --git a/tests/go_service/main.go b/tests/go_service/main.go index 15da351..d5ab7bb 100644 --- a/tests/go_service/main.go +++ b/tests/go_service/main.go @@ -18,7 +18,11 @@ func main() { busyWaitTime := flag.Duration("busy-time", 0, "How long to busy-wait before binding the port") dieAfter := flag.Duration("die-after", 0, "How long to wait before self-destructing") fileToOpen := flag.String("file-to-open", "", "A file to open to check runfiles") - soReuseport := flag.Bool("so-reuseport", false, "If true, sets SO_REUSEPORT when binding the address") + soReuseport := flag.Bool( + "so-reuseport", + os.Getenv("RULES_ITEST_ENABLE_SO_REUSEPORT") == "1", + "If true, sets the platform's reusable-port socket option when binding the address", + ) port := flag.String("port", "", "Port to bind") flag.Parse() diff --git a/tests/go_service/serve_windows.go b/tests/go_service/serve_windows.go index 147819c..6dbe203 100644 --- a/tests/go_service/serve_windows.go +++ b/tests/go_service/serve_windows.go @@ -2,11 +2,38 @@ package main -import "net/http" +import ( + "context" + "log" + "net" + "net/http" + "syscall" + + "golang.org/x/sys/windows" +) func serve(port string, soReuseport bool) { - if soReuseport { - panic("SO_REUSEPORT not supported on Windows!") + lc := net.ListenConfig{ + Control: func(network, address string, conn syscall.RawConn) error { + if !soReuseport { + return nil + } + + var setSockoptErr error + err := conn.Control(func(fd uintptr) { + // FIX: Cast fd directly to syscall.Handle instead of int + setSockoptErr = syscall.SetsockoptInt(syscall.Handle(fd), syscall.SOL_SOCKET, windows.SO_REUSEADDR, 1) + }) + if err != nil { + return err + } + return setSockoptErr + }, + } + + l, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:"+port) + if err != nil { + log.Fatal(err) } - http.ListenAndServe("127.0.0.1:"+port, nil) -} + http.Serve(l, nil) +} \ No newline at end of file diff --git a/tests/so_reuseport/BUILD.bazel b/tests/so_reuseport/BUILD.bazel index ff207ae..301d1ce 100644 --- a/tests/so_reuseport/BUILD.bazel +++ b/tests/so_reuseport/BUILD.bazel @@ -3,15 +3,9 @@ load("@rules_go//go:def.bzl", "go_test") load("@rules_itest//:itest.bzl", "itest_service", "service_test") load("//:must_fail.bzl", "must_fail") -NOT_WINDOWS = select({ - "@platforms//os:windows": ["@platforms//:incompatible"], - "//conditions:default": [], -}) - itest_service( name = "reuseport_service", args = [ - "-so-reuseport", "-port", "$${PORT}", ], @@ -23,7 +17,6 @@ itest_service( "named_port1", ], so_reuseport_aware = True, - target_compatible_with = NOT_WINDOWS, ) itest_service( @@ -38,14 +31,12 @@ itest_service( http_health_check_address = "http://127.0.0.1:$${PORT}", so_reuseport_aware = True, tags = ["manual"], - target_compatible_with = NOT_WINDOWS, ) # TODO(zbarsky): this rule is busted, it isn't actually working correctly. # May need to bust out real bazel integration tests? must_fail( name = "no_reuseport_service_hygiene_test", - target_compatible_with = NOT_WINDOWS, test = "_no_reuseport_service_hygiene_test", )