diff --git a/README.md b/README.md index 529d8ba5..a8d786bb 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,17 @@ unchanged. > Linux 6.1+ · .NET 10 / .NET 11 · experimental +Each reactor is charged against `RLIMIT_MEMLOCK`: measured at 772 KB for the ring at the default +`RingEntries`, plus 64 KB for a 4096-slot buffer ring, so about 836 KB per reactor. The common 8 MB +`ulimit -l` fits roughly ten - which is fewer than the default `ReactorCount` of 12, so a default +server does not start under it. The counter is **per uid**, not per process, so every process you run +shares one budget. A closed ring's memory is reclaimed asynchronously, so standing servers up and +tearing them down in quick succession can hit `ENOMEM` while nothing is leaking; `Ring.Create` +retries briefly before giving up, and names the errno when it does. +> +> Incremental mode charges a further page per live connection, so at the default `MaxConnections` +> a reactor can want 16 MB on top of its ring. + **[Documentation](https://mda2av.github.io/ioxide/)** - architecture, guides, and every example as runnable code side by side. diff --git a/src/ioxide/Native/Native.IoUring.cs b/src/ioxide/Native/Native.IoUring.cs index 05be9647..a2c01ed7 100644 --- a/src/ioxide/Native/Native.IoUring.cs +++ b/src/ioxide/Native/Native.IoUring.cs @@ -69,29 +69,48 @@ public static unsafe partial class Native { public const uint IORING_SETUP_NO_SQARRAY = 1u << 16; public const int EINVAL = 22; + public const int ENOMEM = 12; public const int PROT_READ = 1; public const int PROT_WRITE = 2; public const int MAP_SHARED = 1; public const int MAP_POPULATE = 0x8000; - [DllImport("libc", EntryPoint = "syscall")] + // glibc's syscall() reports failure as -1 with the code in errno; it never returns -errno. So + // all three need SetLastError, and the wrappers below normalise to liburing's convention - a + // negative errno, which is what every caller compares against. Two were declared without it + // (#220), leaving three branches dead: Ring.Create's NO_SQARRAY fallback and the + // EINTR/EAGAIN/EBUSY tolerance in both loops, so one signal ended a reactor. + [DllImport("libc", EntryPoint = "syscall", SetLastError = true)] private static extern long syscall3(long nr, uint a1, IoUringParams* a2); - [DllImport("libc", EntryPoint = "syscall")] + [DllImport("libc", EntryPoint = "syscall", SetLastError = true)] private static extern long syscall6(long nr, uint a1, uint a2, uint a3, uint a4, void* a5, nuint a6); [DllImport("libc", EntryPoint = "syscall", SetLastError = true)] private static extern long syscall4(long nr, uint a1, uint a2, void* a3, uint a4); - public static int io_uring_setup(uint entries, IoUringParams* p) => - (int)syscall3(SYS_IO_URING_SETUP, entries, p); + // Test the long before narrowing: errno is only meaningful on the failure branch. + public static int io_uring_setup(uint entries, IoUringParams* p) + { + long rc = syscall3(SYS_IO_URING_SETUP, entries, p); + + return rc < 0 ? -Marshal.GetLastPInvokeError() : (int)rc; + } + + public static int io_uring_enter(int fd, uint toSubmit, uint minComplete, uint flags) + { + long rc = syscall6(SYS_IO_URING_ENTER, (uint)fd, toSubmit, minComplete, flags, null, 0); + + return rc < 0 ? -Marshal.GetLastPInvokeError() : (int)rc; + } - public static int io_uring_enter(int fd, uint toSubmit, uint minComplete, uint flags) => - (int)syscall6(SYS_IO_URING_ENTER, (uint)fd, toSubmit, minComplete, flags, null, 0); + public static int io_uring_register(int fd, uint opcode, void* arg, uint nrArgs) + { + long rc = syscall4(SYS_IO_URING_REGISTER, (uint)fd, opcode, arg, nrArgs); - public static int io_uring_register(int fd, uint opcode, void* arg, uint nrArgs) => - (int)syscall4(SYS_IO_URING_REGISTER, (uint)fd, opcode, arg, nrArgs); + return rc < 0 ? -Marshal.GetLastPInvokeError() : (int)rc; + } [DllImport("libc")] public static extern void* mmap(void* addr, nuint length, int prot, int flags, int fd, long offset); [DllImport("libc")] public static extern int munmap(void* addr, nuint length); diff --git a/src/ioxide/Reactor/Loop/Reactor.Drainers.cs b/src/ioxide/Reactor/Loop/Reactor.Drainers.cs index 7754c52f..a13fc3c0 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Drainers.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Drainers.cs @@ -1,4 +1,4 @@ -using System.Collections.Concurrent; +using System.Collections.Concurrent; using System.Runtime.CompilerServices; using ioxide.utils; using static ioxide.Native; @@ -16,10 +16,28 @@ public sealed unsafe partial class Reactor #region Wake + // Writers currently holding the eventfd's number, so Teardown can wait them out before it + // closes. Without the gate a writer that read the old number puts 8 bytes into whatever socket + // took it next. Off-reactor callers only, next to a syscall, so the two interlocks are free. + // Reading 0 means the reactor is gone and there is nothing to wake. + private int _wakeUsers; + private void WakeFdWrite() { - ulong v = 1; - write(_wakeFd, &v, 8); // eventfd becomes readable → multishot poll CQE wakes the loop + Interlocked.Increment(ref _wakeUsers); + try + { + int fd = Volatile.Read(ref _wakeFd); + if (fd > 0) + { + ulong v = 1; + write(fd, &v, 8); // eventfd becomes readable → multishot poll CQE wakes the loop + } + } + finally + { + Interlocked.Decrement(ref _wakeUsers); + } } private void ArmWakePoll() diff --git a/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs b/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs index c4006138..ca734f21 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs @@ -65,7 +65,7 @@ private void SetupConnectionBufRing(TcpConnection conn) int ret = io_uring_register(_ring.Fd, IORING_REGISTER_PBUF_RING, ®, 1); if (ret < 0) { - throw new InvalidOperationException($"register pbuf_ring (inc) failed: ret={ret} gid={gid}"); + throw new InvalidOperationException($"register pbuf_ring (inc) failed with errno {-ret}, gid={gid}"); } conn.Bgid = gid; @@ -177,12 +177,19 @@ private void LoopIncremental() RearmStarvedRecvs(); QuicFireDueTimers(); + // The transient three, which the loop carries on from: a signal, the kernel short of + // resources, and overflow entries it could not flush - the CQ drain below is what EBUSY + // wants anyway. Until #220 this could never match (the wrapper returned -1 for every + // failure), so the first signal delivered to a reactor thread ended it. + // + // Anything else is a lifecycle or programming error. Throwing rather than breaking, + // because a reactor that vanishes while the process reports healthy is the worst of + // both; whether the process then dies is the host's call, via Reactor.OnFault. int rc = _ring.SubmitAndWait(1); if (rc < 0 && rc != -EINTR && rc != -EAGAIN && rc != -EBUSY) { - Console.Error.WriteLine($"[r{_id}] io_uring_enter failed: {rc}"); - - break; + throw new InvalidOperationException( + $"[r{_id}] io_uring_enter failed with errno {-rc}; this reactor cannot continue"); } NowMs = Environment.TickCount64; // one read per batch; see Reactor.Tcp.Sweep.cs diff --git a/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs b/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs index 5f52901f..4fc12827 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs @@ -1,4 +1,4 @@ -using System.Runtime.InteropServices; +using System.Runtime.InteropServices; using static ioxide.Native; namespace ioxide; @@ -26,9 +26,7 @@ private void InitSharedRingBuffer() int ret = io_uring_register(_ring.Fd, IORING_REGISTER_PBUF_RING, ®, 1); if (ret < 0) { - int err = Marshal.GetLastPInvokeError(); - - throw new InvalidOperationException($"register pbuf_ring failed: ret={ret} errno={err}"); + throw new InvalidOperationException($"register pbuf_ring failed with errno {-ret}"); } // Slot 0 overlaps the ring's tail field at offset 14; writing only addr/len/bid @@ -56,11 +54,19 @@ private void LoopSharedRing() RearmStarvedRecvs(); QuicFireDueTimers(); + // The transient three, which the loop carries on from: a signal, the kernel short of + // resources, and overflow entries it could not flush - the CQ drain below is what EBUSY + // wants anyway. Until #220 this could never match (the wrapper returned -1 for every + // failure), so the first signal delivered to a reactor thread ended it. + // + // Anything else is a lifecycle or programming error. Throwing rather than breaking, + // because a reactor that vanishes while the process reports healthy is the worst of + // both; whether the process then dies is the host's call, via Reactor.OnFault. int rc = _ring.SubmitAndWait(1); if (rc < 0 && rc != -EINTR && rc != -EAGAIN && rc != -EBUSY) { - Console.Error.WriteLine($"[r{_id}] io_uring_enter failed: {rc}"); - break; + throw new InvalidOperationException( + $"[r{_id}] io_uring_enter failed with errno {-rc}; this reactor cannot continue"); } NowMs = Environment.TickCount64; // one read per batch; see Reactor.Tcp.Sweep.cs diff --git a/src/ioxide/Reactor/Reactor.RingHost.cs b/src/ioxide/Reactor/Reactor.RingHost.cs index bd8f0bea..140ea762 100644 --- a/src/ioxide/Reactor/Reactor.RingHost.cs +++ b/src/ioxide/Reactor/Reactor.RingHost.cs @@ -57,6 +57,18 @@ public T GetService() where T : class /// public Func TcpHandle = null!; + /// + /// Raised on the reactor's own thread when it is ending because of a fault rather than a + /// , after the ring has been torn down. Handle it to log, restart, or bring + /// the process down deliberately. + /// + /// + /// Without a handler the exception propagates out of , which on a bare + /// new Thread(reactor.Run) terminates the process. Which of the two is right is the + /// host's call: losing one shard of N silently is as bad as taking the server down unasked. + /// + public Action? OnFault; + /// /// The per-connection QUIC handler, invoked once per adopted connection (CID demux path). /// Null: no handler is launched (raw engine mode, e.g. a custom diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index ef480d8b..395ebd78 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -1,4 +1,4 @@ -using System.Runtime.InteropServices; +using System.Runtime.InteropServices; using static ioxide.Native; namespace ioxide; @@ -25,6 +25,12 @@ public void Run() BindReactorThread(); _ring = Ring.Create(_ringEntries); + // Covers setup, not just the loop: OnStart is user code, and a throw from it used to leak + // the listener, the eventfd and both ring mappings. Ring.Create stays outside - nothing to + // tear down until it returns. + try + { + // Transports: TCP always; UDP sockets + the QUIC demux only when configured (no-ops otherwise). OpenTcpListeners(); OpenUdpSockets(); @@ -53,10 +59,20 @@ public void Run() StartTicker(); - if (_incremental) LoopIncremental(); - else LoopSharedRing(); - - Teardown(); + _ranLoop = true; + if (_incremental) LoopIncremental(); + else LoopSharedRing(); + } + catch (Exception e) when (OnFault is not null) + { + // Handled by the host, so it does not escape to kill the process. Teardown still runs. + OnFault(this, e); + } + finally + { + // Runs on every exit path: a fatal io_uring_enter, a throw from OnStart, a full SQ. + Teardown(); + } } // Record the owning thread (off-reactor callers detect themselves and go through the handoff @@ -85,18 +101,34 @@ private void AnnounceListening() $" (incremental={_incremental})"); } - // Teardown, still on the reactor thread, in dependency order: sockets close while the ring is - // alive (in-flight ops surface as errors/cancels and are dropped), the ring fd goes next, and - // native memory the kernel could reference (buffer slabs, UDP slot blocks) is freed only after - // that. + /// Set once the loop is entered, so Teardown can tell a failed start from a stop. + private bool _ranLoop; + + // Still on the reactor thread, in dependency order: sockets close while the ring is alive + // (in-flight ops surface as errors/cancels and are dropped), the ring fd goes next, and native + // memory the kernel could reference is freed only after that. private void Teardown() { + bool startFailed = !_ranLoop; + CloseTcpListeners(); TeardownQuic(); CloseUdpFds(); CloseAcceptedTcpSockets(); - close(_wakeFd); + // Taken away before it is closed, and its writers waited out: closing under one would + // free the number and let that writer's 8 bytes land in whatever socket took it next. + // The > 0 test also covers a throw before OpenWakeFd, where this is still 0 - stdin. + int wakeFd = Interlocked.Exchange(ref _wakeFd, 0); + if (wakeFd > 0) + { + SpinWait spin = default; + while (Volatile.Read(ref _wakeUsers) != 0) + { + spin.SpinOnce(); + } + close(wakeFd); + } if (_timerTs != null) { NativeMemory.Free(_timerTs); @@ -108,7 +140,17 @@ private void Teardown() _opTimespecs = null; _opTimespecCapacity = 0; } - _ring.Dispose(); + // Before the fd goes: unregistering is synchronous, closing is not (io_uring_release + // defers to a workqueue), so without this the frees below could hand the kernel's pages + // back to the allocator while it still holds them. + UnregisterSharedBufRings(); + + // Both mappings go back either way; the descriptor is kept when the start failed, because + // releasing its number mid-run kills an unrelated live connection - #242. Bisected: closing + // it by dup2'ing /dev/null over the number is green, so the damage is the number being + // reused, not the ring teardown. Costs the ring's memlock charge until exit, which is what + // this path already did before it tore anything down at all. + _ring.Dispose(closeFd: !startFailed); // Shared provided-buffer ring (incremental mode allocates per connection instead). if (_bufRing != null) @@ -124,6 +166,23 @@ private void Teardown() FreeUdpMemory(); } + // Both are no-ops unless that ring was registered: incremental mode leaves _bufRing null (one + // ring per connection instead), and a reactor with no datagram transport leaves _udpBufRing null. + private void UnregisterSharedBufRings() + { + if (_bufRing != null) + { + var reg = new io_uring_buf_reg { bgid = BgId }; + io_uring_register(_ring.Fd, IORING_UNREGISTER_PBUF_RING, ®, 1); + } + + if (_udpBufRing != null) + { + var reg = new io_uring_buf_reg { bgid = UdpBgId }; + io_uring_register(_ring.Fd, IORING_UNREGISTER_PBUF_RING, ®, 1); + } + } + // Set cross-thread by Stop(); the loops check it at the top of each iteration and exit, after which // Run() tears the ring down on this (the reactor) thread - mandatory for a single-issuer ring. private volatile bool _stopRequested; diff --git a/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs b/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs index 0713035a..1d310234 100644 --- a/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs +++ b/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs @@ -348,6 +348,11 @@ private void OpenTcpListeners() } _listenFds = new int[1 + _tcp.ExtraPorts.Length]; + + // -1, not the default 0: the loop below can throw partway and Teardown runs on that path, + // where an unset slot left at 0 would have it close stdin - handing the number to the next + // socket opened, for a later teardown to shut. + Array.Fill(_listenFds, -1); _listenPorts = new ushort[_listenFds.Length]; _listenPorts[0] = _port; for (int i = 0; i < _tcp.ExtraPorts.Length; i++) @@ -364,7 +369,10 @@ private void CloseTcpListeners() { foreach (int listenFd in _listenFds) { - close(listenFd); + if (listenFd >= 0) + { + close(listenFd); + } } } diff --git a/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs b/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs index 8c56889b..5bb24fec 100644 --- a/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs +++ b/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs @@ -100,6 +100,7 @@ private void OpenUdpSockets() int ports = udpPorts.Length; _udpFds = new int[ports]; + Array.Fill(_udpFds, -1); // unset slots must not read as fd 0; see OpenTcpListeners _udpFdPorts = new ushort[ports]; InitUdpBufRing(); @@ -216,7 +217,7 @@ private void InitUdpBufRing() int ret = io_uring_register(_ring.Fd, IORING_REGISTER_PBUF_RING, ®, 1); if (ret < 0) { - throw new InvalidOperationException($"register udp pbuf_ring failed: ret={ret}"); + throw new InvalidOperationException($"register udp pbuf_ring failed with errno {-ret}"); } // Template: reserved name/control sizes only; iov unused (buffer comes from the ring). diff --git a/src/ioxide/io_uring/Ring.cs b/src/ioxide/io_uring/Ring.cs index cda35d68..cc296b87 100644 --- a/src/ioxide/io_uring/Ring.cs +++ b/src/ioxide/io_uring/Ring.cs @@ -34,26 +34,66 @@ public sealed unsafe class Ring : IDisposable private bool _hasSqArray; + /// ENOMEM here is transient, so it is worth a few milliseconds before giving up. + /// + /// A ring's memory is charged against RLIMIT_MEMLOCK and released ASYNCHRONOUSLY after close, + /// so a host cycling reactors faster than the kernel reclaims them gets ENOMEM while nothing + /// is leaking. Measured: 30,000 create-and-close rings failed 14,788 times, every failure + /// clearing on a retry 5 ms later. Only ENOMEM is retried - every other errno is a decision + /// the kernel has already made. + /// + /// This buys time against a reclaim backlog, not against a limit that is simply too small; for + /// that, the message below names the ring's cost. + /// + private static int SetupWithMemlockRetry(uint entries, IoUringParams* parameters) + { + // 5, 10, 20, 40, 80ms. Sized from the measured reclaim latency of a SINGLE ring - median + // ~20ms, tail 47ms under load - not from a burst, where one ring is always coming back + // within a few ms and a 30ms budget looked sufficient. + const int attempts = 6; + + int fd = io_uring_setup(entries, parameters); + + for (int attempt = 0, delay = 5; fd == -ENOMEM && attempt < attempts - 1; attempt++, delay *= 2) + { + Thread.Sleep(delay); + fd = io_uring_setup(entries, parameters); + } + + return fd; + } + + /// Roughly what one ring costs against RLIMIT_MEMLOCK, for the diagnostic above. + private static int EstimateRingKib(uint entries) + => (int)((entries * (64 + 4) + entries * 2 * 16 + 4096) / 1024); + public static Ring Create(uint entries) { // Prefer NO_SQARRAY (6.6+): the SQ slot index is implicit, dropping one // store + cache line per SQE. Fall back for older kernels (EINVAL). IoUringParams ioUringParams = default; ioUringParams.flags = IORING_SETUP_SINGLE_ISSUER | IORING_SETUP_DEFER_TASKRUN | IORING_SETUP_NO_SQARRAY; - int fd = io_uring_setup(entries, &ioUringParams); + int fd = SetupWithMemlockRetry(entries, &ioUringParams); bool hasSqArray = false; if (fd == -EINVAL) { ioUringParams = default; ioUringParams.flags = IORING_SETUP_SINGLE_ISSUER | IORING_SETUP_DEFER_TASKRUN; - fd = io_uring_setup(entries, &ioUringParams); + fd = SetupWithMemlockRetry(entries, &ioUringParams); hasSqArray = true; } if (fd < 0) { - throw new InvalidOperationException($"io_uring_setup failed: {fd}"); + throw new InvalidOperationException( + $"io_uring_setup failed with errno {-fd}" + + (fd == -ENOMEM + ? $". A ring of {entries} entries costs roughly {EstimateRingKib(entries)} KiB of " + + "RLIMIT_MEMLOCK, and the kernel reclaims a closed ring's memory " + + "asynchronously - raise `ulimit -l`, lower ServerConfig.RingEntries, or " + + "create reactors less abruptly." + : string.Empty)); } var ring = new Ring @@ -178,6 +218,18 @@ public bool TryGetCqe(out IoUringCqe cqe) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void CqAdvance(uint n) => Volatile.Write(ref *_cqHead, *_cqHead + n); + /// + /// Unmaps both rings and, unless says otherwise, closes the + /// descriptor. Keeping it is for one case only - see Reactor.Teardown. + /// + public void Dispose(bool closeFd) + { + _closeFd = closeFd; + Dispose(); + } + + private bool _closeFd = true; + public void Dispose() { if (_ringPtr != null) @@ -190,7 +242,7 @@ public void Dispose() munmap(_sqePtr, _sqeSize); _sqePtr = null; } - if (_fd > 0) + if (_fd > 0 && _closeFd) { close(_fd); _fd = 0; } diff --git a/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs b/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs index 8648040a..96175f98 100644 --- a/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs +++ b/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs @@ -77,6 +77,11 @@ public IoxideConnectionListener(IPEndPoint endpoint, IoxideTransportOptions opti } onReactorStart?.Invoke(r); }; + // One shard of N: losing it should not end the process, but it must not be silent + // either. Without this a fatal errno propagates off a bare Thread and aborts the host. + reactor.OnFault = (r, e) => + Console.Error.WriteLine($"[ioxide] reactor {r.ShardIndex} stopped: {e.Message}"); + _reactors[i] = reactor; _threads[i] = new Thread(reactor.Run) { diff --git a/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs b/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs new file mode 100644 index 00000000..9a5bc8e3 --- /dev/null +++ b/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs @@ -0,0 +1,73 @@ +using System.Net; +using System.Net.Sockets; +using ioxide; + +namespace Ioxide.Tests; + +/// +/// Teardown after a setup that FAILED. Run tears the ring down on every exit path, including a +/// throw from the setup sequence itself, so the fd tables it closes may only be half filled. +/// +/// +/// The tables are new int[n], so an unset slot reads as 0 - stdin, not something this +/// library opened. Closing it is the opposite of a leak: the number goes back to the process, the +/// next socket opened takes it, and the next teardown to close 0 shuts somebody else's live +/// connection. Same for _wakeFd, which OpenWakeFd sets only near the end of setup. +/// +/// /proc/self/fd/0 answers whether fd 0 is open without a P/Invoke: it exists while it does. +/// +internal static class ReactorSetupTeardownTests +{ + public static void Register(Runner runner) + { + runner.Test("reactor: a bind that fails tears down without closing stdin", () => + { + Assert.True(File.Exists("/proc/self/fd/0"), "fd 0 must be open before the test says anything"); + + // A SO_REUSEPORT bind is refused unless EVERY socket on the port asked for it, and a + // plain listener asks for nothing - so this bind fails with EADDRINUSE, deterministically + // and without privilege. The throw lands in OpenTcpListeners, which runs before + // OpenWakeFd: _listenFds[0] and _wakeFd are both still 0 when the finally tears down. + using var blocker = new TcpListener(IPAddress.Any, 0); + blocker.Start(); + int port = ((IPEndPoint)blocker.LocalEndpoint).Port; + + var config = new ServerConfig + { + ReactorCount = 1, + Tcp = new TcpOptions { Port = (ushort)port }, + }; + var reactor = new Reactor(0, config) + { + TcpHandle = (_, connection) => + { + connection.DecRef(); + return Task.CompletedTask; + }, + }; + + Exception? failure = null; + var thread = new Thread(() => + { + try + { + reactor.Run(); + } + catch (Exception e) + { + failure = e; + } + }); + thread.Start(); + + Assert.True(thread.Join(TimeSpan.FromSeconds(10)), + "the reactor should have failed its bind and returned, not parked in the loop"); + Assert.True(failure is not null, + "binding a port already held without SO_REUSEPORT must fail the reactor"); + + Assert.True(File.Exists("/proc/self/fd/0"), + "teardown after the failed bind closed fd 0 - stdin - and the number is now free " + + "for the next socket the process opens"); + }); + } +} diff --git a/tests/Ioxide.Tests.E2E/Program.cs b/tests/Ioxide.Tests.E2E/Program.cs index 2e4f009e..3dd57f72 100644 --- a/tests/Ioxide.Tests.E2E/Program.cs +++ b/tests/Ioxide.Tests.E2E/Program.cs @@ -18,6 +18,7 @@ private static int Main() RecvBufferReclaimTests.Register(runner); PipeReaderContractTests.Register(runner); TcpTimeoutTests.Register(runner); + ReactorSetupTeardownTests.Register(runner); UdpTests.Register(runner); QuicTests.Register(runner); QuicEngineTests.Register(runner); diff --git a/tests/Ioxide.Tests.Harness/TestServer.cs b/tests/Ioxide.Tests.Harness/TestServer.cs index e2ddb4cd..541e03c5 100644 --- a/tests/Ioxide.Tests.Harness/TestServer.cs +++ b/tests/Ioxide.Tests.Harness/TestServer.cs @@ -627,6 +627,19 @@ public static int StartQuicServingH3Driver( // report the real reason instead of "never started listening". private static readonly System.Collections.Concurrent.ConcurrentDictionary StartupFailures = new(); + /// + /// Ports whose death a test has already accounted for, so a late record cannot be re-reported + /// as unowned. + /// + /// + /// Ordering, not speed. A reactor whose OnStart throws faults `started` at once but reaches + /// StartupFailures only after Run unwinds - and Run now tears the ring down on the way, which + /// takes milliseconds. So the consumer can look before the producer writes, remove nothing, + /// and the entry lands with nobody left to claim it. Marking the port closes that either way. + /// ReserveFreePort only moves forward, so a port keys one server's lifetime. + /// + private static readonly System.Collections.Concurrent.ConcurrentDictionary ObservedDeaths = new(); + /// /// Reactor.Run as a thread body, with the exception caught. Without this a bind or listen /// failure is unhandled on a background thread and .NET terminates the process - so a single @@ -641,7 +654,11 @@ private static ThreadStart RunGuarded(Reactor reactor, int port) => () => } catch (Exception e) { - StartupFailures[port] = e; + // Not recorded if a test already accounted for this death - see ObservedDeaths. + if (!ObservedDeaths.ContainsKey(port)) + { + StartupFailures[port] = e; + } // ALWAYS print, even though WaitForListen may also report it. Only failures that happen // before the listener is up are ever consumed there, and Run opens the listener before @@ -696,6 +713,7 @@ private static void WaitForOnStart(int port, TaskCompletionSource started) // has to be looked for rather than waited on. if (StartupFailures.TryRemove(port, out Exception? failure)) { + ObservedDeaths[port] = 1; throw new Exception($"server on :{port} failed to start: {failure.Message}", failure); } @@ -708,7 +726,10 @@ private static void WaitForOnStart(int port, TaskCompletionSource started) } catch (AggregateException e) when (e.InnerException is not null) { - StartupFailures.TryRemove(port, out _); // consumed here, so it is not also reported unowned + // Claimed BEFORE removing: the reactor may not have recorded it yet, because the + // throw still has to unwind through Run's teardown. + ObservedDeaths[port] = 1; + StartupFailures.TryRemove(port, out _); throw new Exception($"server on :{port} failed to start: {e.InnerException.Message}", e.InnerException); } } diff --git a/tests/Ioxide.Tests.Unit/Program.cs b/tests/Ioxide.Tests.Unit/Program.cs index f8636f77..d0ae09a0 100644 --- a/tests/Ioxide.Tests.Unit/Program.cs +++ b/tests/Ioxide.Tests.Unit/Program.cs @@ -12,6 +12,7 @@ private static int Main() var runner = new Runner(); VersionTests.Register(runner); + SyscallErrnoTests.Register(runner); DemuxParseTests.Register(runner); MessageTests.Register(runner); ResponseCapTests.Register(runner); diff --git a/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs b/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs new file mode 100644 index 00000000..40b638a4 --- /dev/null +++ b/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs @@ -0,0 +1,53 @@ +using ioxide; + +namespace Ioxide.Tests; + +/// +/// The io_uring syscall wrappers must report failures the way the rest of the code reads them: +/// as a negative errno, liburing's convention (#220). +/// +/// +/// glibc's syscall() returns -1 and puts the code in errno; it never returns -errno, so an +/// import without SetLastError loses it and every comparison against a specific errno goes +/// dead - NO_SQARRAY never falls back on 6.1-6.5, and one interrupted io_uring_enter ends that +/// reactor while the process reports healthy at reduced capacity. +/// +/// The three declarations sat together and only syscall4 had it, which is how it went unnoticed. +/// No sockets, no signals, no timing here: ask the kernel for something it must refuse. +/// +internal static class SyscallErrnoTests +{ + /// EINVAL, which io_uring_setup must return for a zero-entry ring. + private const int ExpectedSetupErrno = -22; + + /// EBADF, which io_uring_enter must return for a descriptor that is not a ring. + private const int ExpectedEnterErrno = -9; + + public static void Register(Runner runner) + { + runner.Test("syscall: io_uring_setup reports a negative errno, not -1", () => + { + // Refused by every kernel, so no feature detection and no accidental pass. + int rc = SetupZeroEntries(); + + Assert.True(rc < 0, $"io_uring_setup(0) was expected to fail, got {rc}"); + Assert.Equal(ExpectedSetupErrno, rc); + }); + + runner.Test("syscall: io_uring_enter reports a negative errno, not -1", () => + { + // Not a ring descriptor, so the kernel answers EBADF - the value the two loops test + // against -EINTR/-EAGAIN/-EBUSY to decide whether to keep going. + int rc = Native.io_uring_enter(-1, 0, 0, 0); + + Assert.True(rc < 0, $"io_uring_enter(-1) was expected to fail, got {rc}"); + Assert.Equal(ExpectedEnterErrno, rc); + }); + } + + private static unsafe int SetupZeroEntries() + { + Native.IoUringParams parameters = default; + return Native.io_uring_setup(0, ¶meters); + } +}