From 3b6906a75479c8059055a3a96f28c6a44548906e Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Sun, 20 Sep 2026 23:35:31 +0100 Subject: [PATCH 01/16] ring: read errno, so the transient-failure handling actually works glibc's syscall() reports failure as -1 with the code in errno; it never returns -errno. syscall3 and syscall6 were declared without SetLastError, so every io_uring_setup and io_uring_enter failure arrived as -1 and three comparisons against specific errnos were dead code (#220): Ring.cs:46 if (fd == -EINVAL) the pre-6.6 fallback Reactor.Loop.SharedRing.cs:60 EINTR/EAGAIN/EBUSY transient tolerance Reactor.Loop.Incremental.cs:181 EINTR/EAGAIN/EBUSY transient tolerance The second and third are worse than inert. They exist to let the loop survive an interrupted enter, and instead guaranteed the opposite: one signal delivered to a reactor thread - any PosixSignalRegistration the application installs, a profiler's SIGPROF, SIGHUP for config reload - returned -1, matched none of the exclusions, and ended that reactor. The process kept running with one reactor fewer and nothing thrown, logged as anything but a stray line, or otherwise distinguishable from a clean Stop(). Measured before the fix: io_uring_setup(0) returned -1 where -22 was expected, io_uring_enter(-1) returned -1 where -9 was expected. Both are now tests. The third declaration of the same libc function, syscall4 for io_uring_register, already had SetLastError and its callers already read the errno. Only two of the three were wrong. Also here, because the same investigation turned them up: Run() had no try/finally, so anything thrown out of the loop skipped Teardown() and leaked the ring fd, both mmaps, the eventfd and the buffer slab. Ring memory is charged against RLIMIT_MEMLOCK, so leaking rings is precisely how a long-lived host ends up unable to create one. A fatal errno now throws instead of breaking quietly. A reactor that vanishes while the process reports healthy is the worst of the options; the finally tears the ring down and the exception reaches whoever started the thread. Transients still just carry on - and now genuinely do. Ring.Create retries ENOMEM. A ring's memory is released ASYNCHRONOUSLY after close, so a host that creates and drops reactors faster than the kernel reclaims them gets ENOMEM with nothing leaking. Measured: 30,000 create-and-close cycles failed 14,788 times, and every failure cleared on a 5ms retry. This is what aborts the GenHTTP acceptance suite partway through, where it reported only "io_uring_setup failed: -1". It now retries, and on giving up names the errno and the memlock cost. Not addressed: the NO_SQARRAY fallback is now reachable for the first time, which means Ring.cs's _sqArray path has never executed anywhere. It looks right, but 6.1-6.5 has no coverage and nothing forces the fallback in CI. Bench, Tcp/Raw at 4 reactors, four alternating pairs: -4.6, +3.3, -0.5, +2.2, mean +0.15%, two up two down. SetLastError costs about 10ns per enter, which at a realistic completion batch is ~0.6ns per request. E2E 194, Unit 48, Http 44, Tls 142, Chaos 47, File 4. --- README.md | 5 ++ src/ioxide/Native/Native.IoUring.cs | 29 ++++++-- .../Reactor/Loop/Reactor.Loop.Incremental.cs | 15 ++++- .../Reactor/Loop/Reactor.Loop.SharedRing.cs | 14 +++- src/ioxide/Reactor/Reactor.Runner.cs | 18 +++-- src/ioxide/io_uring/Ring.cs | 42 +++++++++++- tests/Ioxide.Tests.Unit/Program.cs | 1 + tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs | 67 +++++++++++++++++++ 8 files changed, 173 insertions(+), 18 deletions(-) create mode 100644 tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs diff --git a/README.md b/README.md index 529d8ba5..a2fe3779 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,11 @@ unchanged. > Linux 6.1+ · .NET 10 / .NET 11 · experimental +Each reactor's ring is charged against `RLIMIT_MEMLOCK` - roughly 700 KB at the default +`RingEntries`, so the common 8 MB `ulimit -l` fits about ten of them. A closed ring's memory comes +back asynchronously, so standing many servers up and tearing them down in quick succession can hit +`ENOMEM` while nothing is leaking; `Ring.Create` retries briefly before giving up, and says so. + **[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..f681a5f8 100644 --- a/src/ioxide/Native/Native.IoUring.cs +++ b/src/ioxide/Native/Native.IoUring.cs @@ -69,26 +69,43 @@ 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")] + // All three go through glibc's syscall(), which reports failure as -1 with the code in errno - + // it never returns -errno. So every one of them needs SetLastError, and the wrappers below + // normalise to liburing's convention: a negative errno, which is what every caller compares + // against. Two of these were declared without it (#220), which made three branches dead code: + // Ring.Create's fallback for kernels without IORING_SETUP_NO_SQARRAY, and the EINTR/EAGAIN/EBUSY + // tolerance in both reactor loops - so one signal delivered to a reactor thread ended it. + [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: a successful io_uring_enter returns a submission count, and + // 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); - 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); + return rc < 0 ? -Marshal.GetLastPInvokeError() : (int)rc; + } 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); diff --git a/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs b/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs index c4006138..a34b424b 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs @@ -177,12 +177,21 @@ private void LoopIncremental() RearmStarvedRecvs(); QuicFireDueTimers(); + // These three are the transient ones and the loop simply carries on: EINTR is a signal, + // EAGAIN is the kernel short of resources, EBUSY means overflow entries could not be + // flushed - and the fall-through below drains the CQ, which is exactly what EBUSY wants. + // Until #220 this comparison could never match, because 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 (EBADF, EINVAL, ENXIO on a dying + // ring). Throwing rather than breaking, because a reactor that vanishes while the + // process keeps reporting healthy is the worst of both: Run's finally tears the ring + // down and the exception reaches whoever started the thread. 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..fbed391b 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs @@ -56,11 +56,21 @@ private void LoopSharedRing() RearmStarvedRecvs(); QuicFireDueTimers(); + // These three are the transient ones and the loop simply carries on: EINTR is a signal, + // EAGAIN is the kernel short of resources, EBUSY means overflow entries could not be + // flushed - and the fall-through below drains the CQ, which is exactly what EBUSY wants. + // Until #220 this comparison could never match, because 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 (EBADF, EINVAL, ENXIO on a dying + // ring). Throwing rather than breaking, because a reactor that vanishes while the + // process keeps reporting healthy is the worst of both: Run's finally tears the ring + // down and the exception reaches whoever started the thread. 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.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index ef480d8b..02807097 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -53,10 +53,20 @@ public void Run() StartTicker(); - if (_incremental) LoopIncremental(); - else LoopSharedRing(); - - Teardown(); + // Teardown in a finally, not after the loop: anything thrown out of the loop - a fatal + // io_uring_enter, GetSqeOrFlush giving up on a full SQ, a handler fault that escapes - + // otherwise skipped it and leaked the ring fd, both mmaps, the eventfd and the buffer slab. + // Ring memory is charged against RLIMIT_MEMLOCK, so leaking rings is how a long-lived host + // eventually cannot create any. + try + { + if (_incremental) LoopIncremental(); + else LoopSharedRing(); + } + finally + { + Teardown(); + } } // Record the owning thread (off-reactor callers detect themselves and go through the handoff diff --git a/src/ioxide/io_uring/Ring.cs b/src/ioxide/io_uring/Ring.cs index cda35d68..8dfbe964 100644 --- a/src/ioxide/io_uring/Ring.cs +++ b/src/ioxide/io_uring/Ring.cs @@ -34,26 +34,62 @@ 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 that creates and drops reactors faster than the kernel reclaims them - a test + /// suite standing servers up and tearing them down is the usual shape - gets ENOMEM while + /// nothing is actually leaking. Measured: creating and immediately closing 30,000 rings failed + /// 14,788 times, and every failure cleared on a retry 5 ms later. + /// + /// Only ENOMEM is retried. Every other errno is a decision the kernel has already made. + /// + private static int SetupWithMemlockRetry(uint entries, IoUringParams* parameters) + { + const int attempts = 6; + + int fd = io_uring_setup(entries, parameters); + + for (int attempt = 1; fd == -ENOMEM && attempt < attempts; attempt++) + { + Thread.Sleep(attempt * 2); // 2, 4, 6, 8, 10ms - ~30ms total, well past what we measured + 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 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..cc67e4c7 --- /dev/null +++ b/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs @@ -0,0 +1,67 @@ +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. An +/// import declared without SetLastError therefore loses the code entirely, and every +/// comparison against a specific errno becomes dead: +/// +/// +/// Ring.cs:46 if (fd == -EINVAL) // the pre-6.6 fallback +/// Reactor.Loop.SharedRing.cs:60 rc != -EINTR && rc != -EAGAIN && rc != -EBUSY +/// Reactor.Loop.Incremental.cs:181 rc != -EINTR && rc != -EAGAIN && rc != -EBUSY +/// +/// +/// So NO_SQARRAY never falls back on 6.1-6.5, and one interrupted io_uring_enter - a SIGHUP +/// handler, a profiler's SIGPROF, anything the kernel routes to a reactor thread - ends that +/// reactor, while the process carries on reporting healthy at reduced capacity. +/// +/// The three declarations sit next to each other in Native.IoUring.cs and only one of them, +/// syscall4 for io_uring_register, is declared SetLastError = true. Its callers read +/// Marshal.GetLastPInvokeError and work correctly; the other two are the bug. +/// +/// No sockets, no signals, no timing here: ask the kernel for something it must refuse and read +/// what comes back. +/// +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", () => + { + // entries == 0 is refused by every kernel, so this needs no feature detection and + // cannot pass by accident on a machine that happens to support something. + 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", () => + { + // -1 is not a ring descriptor, so the kernel answers EBADF. This is the value the two + // loops compare 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); + } +} From d1d29a4a6dfc7727e1c47658e9399d522a2ae59e Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Sun, 20 Sep 2026 23:38:08 +0100 Subject: [PATCH 02/16] README: the measured memlock charge, and that the counter is per uid 772 KB for the ring at the default RingEntries plus 64 KB for a 4096-slot buffer ring, measured rather than estimated. The per-uid part matters more than the number: locked_vm is shared across every process of the same user, so a second program churning rings eats the same budget. --- README.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index a2fe3779..e1a9ae9b 100644 --- a/README.md +++ b/README.md @@ -22,10 +22,12 @@ unchanged. > Linux 6.1+ · .NET 10 / .NET 11 · experimental -Each reactor's ring is charged against `RLIMIT_MEMLOCK` - roughly 700 KB at the default -`RingEntries`, so the common 8 MB `ulimit -l` fits about ten of them. A closed ring's memory comes -back asynchronously, so standing many servers up and tearing them down in quick succession can hit -`ENOMEM` while nothing is leaking; `Ring.Create` retries briefly before giving up, and says so. +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. 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. **[Documentation](https://mda2av.github.io/ioxide/)** - architecture, guides, and every example as runnable code side by side. From ac8e0a31df3ff421e53a7d418ee9fde43434eb18 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Sun, 20 Sep 2026 23:54:46 +0100 Subject: [PATCH 03/16] review: cover the setup path, give hosts a fault seam, normalise register Three findings from review, two of them defects in the previous commit. The try started at the LOOP, which is not where the leak is. Ring.Create, OpenTcpListeners, InitSharedRingBuffer, OpenWakeFd and OnStart - user code - all sat outside it, so a throw from any of them skipped Teardown entirely. The comment named exactly what leaks and then failed to cover it. The test harness throws from OnStart deliberately, in every E2E run: measured at three descriptors and ~745 KiB of RLIMIT_MEMLOCK per failed start, 50 reactors leaking 162 fds. The try now opens immediately after Ring.Create. Throwing a fatal errno aborted the process under the shipped host. ioxide.Kestrel starts each reactor with a bare new Thread(reactor.Run) and catches nothing, so what had been one dead reactor became SIGABRT and a core dump for the whole server. The comment justifying the throw claimed "the exception reaches whoever started the thread", which was false for this repo's own host. Reactor.OnFault is the seam. Unhandled, the exception still propagates and a bare Thread still ends the process - which reactor is right is the host's call, not this library's. ioxide.Kestrel now handles it and logs: losing one shard of N should not end the server, but it must not be silent either, which is the whole complaint behind #220. io_uring_register was left returning -1 three lines under a comment saying all three wrappers normalise. Latent, since no caller compared it to a specific errno, but it is #220's exact shape left in the file written to fix #220. Both pbuf_ring registration sites and the UDP one now print the errno instead of -1. Also: the README said an 8 MB memlock fits about ten reactors without noticing that the default ReactorCount is 12, so a default server does not start under it. And Ring.Create's retry remarks now say what it does not buy - under a genuinely small limit a third of attempts still fail first time, and the answer there is a bigger limit or a smaller ring. E2E 194, Unit 48, Http 44, Tls 142, Chaos 47, File 4. --- README.md | 3 ++- src/ioxide/Native/Native.IoUring.cs | 8 +++++-- .../Reactor/Loop/Reactor.Loop.Incremental.cs | 7 +++--- .../Reactor/Loop/Reactor.Loop.SharedRing.cs | 9 ++++--- src/ioxide/Reactor/Reactor.RingHost.cs | 14 +++++++++++ src/ioxide/Reactor/Reactor.Runner.cs | 24 +++++++++++++------ .../Reactor/Transport/Udp/Reactor.Udp.cs | 2 +- src/ioxide/io_uring/Ring.cs | 5 ++++ .../IoxideConnectionListener.cs | 6 +++++ 9 files changed, 59 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index e1a9ae9b..b60f72f6 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,8 @@ unchanged. 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. The counter is **per uid**, not per process, so every process you run +`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. diff --git a/src/ioxide/Native/Native.IoUring.cs b/src/ioxide/Native/Native.IoUring.cs index f681a5f8..ff3555ff 100644 --- a/src/ioxide/Native/Native.IoUring.cs +++ b/src/ioxide/Native/Native.IoUring.cs @@ -107,8 +107,12 @@ public static int io_uring_enter(int fd, uint toSubmit, uint minComplete, uint f return rc < 0 ? -Marshal.GetLastPInvokeError() : (int)rc; } - 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); + 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); + + 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.Loop.Incremental.cs b/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs index a34b424b..45bbccda 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; @@ -185,8 +185,9 @@ private void LoopIncremental() // // Anything else is a lifecycle or programming error (EBADF, EINVAL, ENXIO on a dying // ring). Throwing rather than breaking, because a reactor that vanishes while the - // process keeps reporting healthy is the worst of both: Run's finally tears the ring - // down and the exception reaches whoever started the thread. + // process keeps reporting healthy is the worst of both. Run's finally tears the ring + // down; whether the process then dies is the host's decision, via Reactor.OnFault - + // on a bare Thread with no handler, an unhandled exception ends the process. int rc = _ring.SubmitAndWait(1); if (rc < 0 && rc != -EINTR && rc != -EAGAIN && rc != -EBUSY) { diff --git a/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs b/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs index fbed391b..9a24ed75 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs @@ -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 @@ -64,8 +62,9 @@ private void LoopSharedRing() // // Anything else is a lifecycle or programming error (EBADF, EINVAL, ENXIO on a dying // ring). Throwing rather than breaking, because a reactor that vanishes while the - // process keeps reporting healthy is the worst of both: Run's finally tears the ring - // down and the exception reaches whoever started the thread. + // process keeps reporting healthy is the worst of both. Run's finally tears the ring + // down; whether the process then dies is the host's decision, via Reactor.OnFault - + // on a bare Thread with no handler, an unhandled exception ends the process. int rc = _ring.SubmitAndWait(1); if (rc < 0 && rc != -EINTR && rc != -EAGAIN && rc != -EBUSY) { diff --git a/src/ioxide/Reactor/Reactor.RingHost.cs b/src/ioxide/Reactor/Reactor.RingHost.cs index bd8f0bea..558ef57b 100644 --- a/src/ioxide/Reactor/Reactor.RingHost.cs +++ b/src/ioxide/Reactor/Reactor.RingHost.cs @@ -57,6 +57,20 @@ 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 . On a bare + /// new Thread(reactor.Run) - which is how every sample and ioxide.Kestrel start + /// one - that terminates the process. Which of the two is right is the host's call, not this + /// library's: losing one shard of N silently is indefensible, and so is taking the whole server + /// down without being asked. + /// + 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 02807097..d3423435 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -25,6 +25,14 @@ public void Run() BindReactorThread(); _ring = Ring.Create(_ringEntries); + // The try opens HERE, not at the loop: setup is where the leak actually happens. OnStart is + // user code and the test harness deliberately throws from it, which leaked the ring fd, the + // listener and the eventfd - three descriptors and ~745 KiB of RLIMIT_MEMLOCK - on every + // failed start. Ring.Create itself is outside because there is 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,18 +61,20 @@ public void Run() StartTicker(); - // Teardown in a finally, not after the loop: anything thrown out of the loop - a fatal - // io_uring_enter, GetSqeOrFlush giving up on a full SQ, a handler fault that escapes - - // otherwise skipped it and leaked the ring fd, both mmaps, the eventfd and the buffer slab. - // Ring memory is charged against RLIMIT_MEMLOCK, so leaking rings is how a long-lived host - // eventually cannot create any. - try - { 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 { + // Whatever happened - a fatal io_uring_enter, a throw from OnStart, GetSqeOrFlush + // giving up on a full SQ - the ring fd, both mmaps, the eventfd and the buffer slab go + // back. Ring memory is charged against RLIMIT_MEMLOCK, so leaking rings is how a + // long-lived host eventually cannot create one. Teardown(); } } diff --git a/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs b/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs index 8c56889b..a8d1f5d4 100644 --- a/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs +++ b/src/ioxide/Reactor/Transport/Udp/Reactor.Udp.cs @@ -216,7 +216,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 8dfbe964..f6804596 100644 --- a/src/ioxide/io_uring/Ring.cs +++ b/src/ioxide/io_uring/Ring.cs @@ -43,6 +43,11 @@ public sealed unsafe class Ring : IDisposable /// 14,788 times, and every failure cleared 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: with + /// an 8 MB RLIMIT_MEMLOCK and the default ring size, measured, a third of attempts still fail + /// first time and a few per thousand exhaust all six. There the answer is a bigger limit or a + /// smaller ring, and the message below says so. /// private static int SetupWithMemlockRetry(uint entries, IoUringParams* parameters) { diff --git a/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs b/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs index 8648040a..83e5fbd1 100644 --- a/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs +++ b/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs @@ -77,6 +77,12 @@ public IoxideConnectionListener(IPEndPoint endpoint, IoxideTransportOptions opti } onReactorStart?.Invoke(r); }; + // A reactor is one shard of N. Losing one should not end the process, but it must not + // be silent either - without this, a fatal io_uring 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) { From 03a839ece6683024bdf776782eb4e398a62814c5 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 00:34:51 +0100 Subject: [PATCH 04/16] review: size the retry to the measured reclaim latency, not to a burst Second review pass, all four from measurement. The retry budget was sized from the wrong experiment. 2/4/6/8/10ms came from a BURST of 30,000 create-and-close cycles, where many rings are in flight and one is always coming back within a few ms. A restarting server produces the one-at-a-time case instead, and a single ring's reclaim has a median around 20ms with a tail to 47 under load - so 30ms still failed a fifth of the time. Now 5/10/20/40/80, about 155ms, which costs nothing on the success path. The README reasoned about 8 MB while incremental mode charges another page per live connection: at the default MaxConnections that is up to 16 MB per reactor on top of the ring, twice the figure the paragraph was arguing from. _wakeFd was closed without being zeroed. WakeFdWrite runs from any thread and guards only on _wakeFd > 0, so the stale number let a write land in whatever fd reused it. Latent before, because Teardown only followed an explicit Stop(); it can now follow a fault at an arbitrary instant while a host is still handing work in, which is the change that makes it worth closing. And the new test's own remark claimed the io_uring_register callers read the errno and work correctly. Two of the three printed ret=-1 with no errno at all. They are normalised now, so the remark says what is true. Verified in review, worth recording: on main a reactor death is invisible to the test harness - Run() returns normally and RunGuarded has nothing to record. With the throw, the harness reports it and Summary fails the run. Teardown completes before the process aborts, confirmed by strace: every close and munmap executes, then SIGABRT. E2E 194, Unit 48, Http 44, Tls 142, Chaos 47, File 4. --- README.md | 3 +++ src/ioxide/Reactor/Reactor.Runner.cs | 5 +++++ src/ioxide/io_uring/Ring.cs | 9 +++++++-- tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs | 6 +++--- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b60f72f6..a8d786bb 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,9 @@ server does not start under it. The counter is **per uid**, not per process, so 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/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index d3423435..97242caf 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -116,7 +116,12 @@ private void Teardown() CloseUdpFds(); CloseAcceptedTcpSockets(); + // Zeroed, not just closed: WakeFdWrite is called from any thread and guards only on + // _wakeFd > 0, so leaving the old number here writes into whatever fd reused it. Teardown + // used to follow an explicit Stop(); it can now also follow a fault, at any instant, while + // a host is still handing work in. close(_wakeFd); + _wakeFd = 0; if (_timerTs != null) { NativeMemory.Free(_timerTs); diff --git a/src/ioxide/io_uring/Ring.cs b/src/ioxide/io_uring/Ring.cs index f6804596..45d23b2d 100644 --- a/src/ioxide/io_uring/Ring.cs +++ b/src/ioxide/io_uring/Ring.cs @@ -51,13 +51,18 @@ public sealed unsafe class Ring : IDisposable /// private static int SetupWithMemlockRetry(uint entries, IoUringParams* parameters) { + // 5, 10, 20, 40, 80ms - about 155ms in total. Sized from the measured reclaim latency of a + // SINGLE ring, whose median is ~20ms and whose tail reaches 47ms under load. A shorter + // schedule looked sufficient against a BURST, where many rings are in flight and one is + // always coming back within a few ms, but the one-at-a-time case a restarting server + // produces is far slower and a 30ms budget still failed a fifth of the time. const int attempts = 6; int fd = io_uring_setup(entries, parameters); - for (int attempt = 1; fd == -ENOMEM && attempt < attempts; attempt++) + for (int attempt = 0, delay = 5; fd == -ENOMEM && attempt < attempts - 1; attempt++, delay *= 2) { - Thread.Sleep(attempt * 2); // 2, 4, 6, 8, 10ms - ~30ms total, well past what we measured + Thread.Sleep(delay); fd = io_uring_setup(entries, parameters); } diff --git a/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs b/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs index cc67e4c7..98e86f89 100644 --- a/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs +++ b/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs @@ -21,9 +21,9 @@ namespace Ioxide.Tests; /// handler, a profiler's SIGPROF, anything the kernel routes to a reactor thread - ends that /// reactor, while the process carries on reporting healthy at reduced capacity. /// -/// The three declarations sit next to each other in Native.IoUring.cs and only one of them, -/// syscall4 for io_uring_register, is declared SetLastError = true. Its callers read -/// Marshal.GetLastPInvokeError and work correctly; the other two are the bug. +/// The three declarations sat next to each other in Native.IoUring.cs and only one of them, +/// syscall4 for io_uring_register, was declared SetLastError = true - which is how the omission +/// went unnoticed. All three now normalise to a negative errno. /// /// No sockets, no signals, no timing here: ask the kernel for something it must refuse and read /// what comes back. From 245a4325a4f31d7892097edb23e8a7c4f4038249 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 09:58:30 +0100 Subject: [PATCH 05/16] tests: claim a reactor death by port, not by winning a race CI caught what three local runs did not. Every test passed and the run still failed: [test-reactor:32277] died: System.InvalidOperationException: refused-on-purpose FAIL a test reactor died and no test observed it: :32277 - refused-on-purpose The harness records a death in StartupFailures only once Run has fully unwound, but the OnStart wrapper faults `started` immediately and then throws. WaitForOnStart consumes via the faulted task and removes the entry - so the two have always been in a race, and the harness's own comment admits it: "Every refusal test in the suite depended on losing that race the right way round." Teardown now runs on the way out of that throw, which is the point of the change this branch makes, and it takes milliseconds. So the consumer started winning: it removed nothing, and the record landed afterwards with nobody left to claim it. Fixed by claiming the PORT rather than the entry. WaitForOnStart marks the port observed on both consume paths, before removing; RunGuarded skips recording for a port already claimed, and still always prints. Whichever side goes first, the death is accounted for exactly once. Ports never repeat within a run, so the port is a sound key for one server's life. E2E 194 x3, Unit 48, Http 44, Tls 142, Chaos 47, File 4. --- tests/Ioxide.Tests.Harness/TestServer.cs | 28 ++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/Ioxide.Tests.Harness/TestServer.cs b/tests/Ioxide.Tests.Harness/TestServer.cs index e2ddb4cd..9ea175f0 100644 --- a/tests/Ioxide.Tests.Harness/TestServer.cs +++ b/tests/Ioxide.Tests.Harness/TestServer.cs @@ -627,6 +627,22 @@ 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, is what this fixes. A reactor whose OnStart throws faults `started` + /// immediately but is only recorded in StartupFailures once Run has fully unwound - and Run now + /// tears the ring down on the way out, which takes milliseconds. So the consumer below can look + /// before the producer writes, remove nothing, and the entry then lands with nobody left to + /// claim it. Marking the port closes the race whichever way round it goes. + /// + /// Ports are never reused within a run (ReserveFreePort only moves forward), so the port is a + /// sound key for 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 +657,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 +716,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 +729,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); } } From 2fa156a0de7e012db4a7e5bb69fe6014407a03b2 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 10:25:59 +0100 Subject: [PATCH 06/16] reactor: never close fd 0 when setup fails before the tables are filled Teardown runs on the setup-failure path now, and the fd tables it closes may only be half filled. _listenFds and _udpFds are new int[n], so an unset slot reads as 0, and _wakeFd is still its default 0 until OpenWakeFd runs near the end of setup - later than OpenTcpListeners, OpenUdpSockets, InitQuic and the buffer-ring init, any of which can throw. Closing 0 is worse than a leak. It shuts stdin and hands the number back, so the next socket the process opens becomes fd 0 - and the next teardown to close 0 shuts a live connection belonging to somebody else. The arrays are filled with -1 so an unset slot is unambiguous, and the two close loops plus the wake fd skip anything negative or zero. Proven by the new E2E test: a plain TcpListener sets no SO_REUSEPORT, so the reactor's bind to that port is refused with EADDRINUSE - a deterministic, unprivileged failure inside OpenTcpListeners. Reverting the fix fails it (194 passed, 1 failed); with the fix the suite is 195 passed, 0 failed. --- src/ioxide/Reactor/Reactor.Runner.cs | 18 +++-- .../Reactor/Transport/Tcp/Reactor.Tcp.cs | 11 ++- .../Reactor/Transport/Udp/Reactor.Udp.cs | 1 + .../Core/ReactorSetupTeardownTests.cs | 76 +++++++++++++++++++ tests/Ioxide.Tests.E2E/Program.cs | 1 + 5 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index 97242caf..77a0740b 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; @@ -116,11 +116,17 @@ private void Teardown() CloseUdpFds(); CloseAcceptedTcpSockets(); - // Zeroed, not just closed: WakeFdWrite is called from any thread and guards only on - // _wakeFd > 0, so leaving the old number here writes into whatever fd reused it. Teardown - // used to follow an explicit Stop(); it can now also follow a fault, at any instant, while - // a host is still handing work in. - close(_wakeFd); + // Guarded because OpenWakeFd runs late in setup and Teardown now also follows a throw + // from anything before it, where _wakeFd is still its default 0 - stdin, not an eventfd. + // + // Zeroed rather than merely closed: WakeFdWrite is called from any thread and guards only + // on _wakeFd > 0, so leaving the old number here writes into whatever fd reused it. + // Teardown used to follow an explicit Stop(); it can now also follow a fault, at any + // instant, while a host is still handing work in. + if (_wakeFd > 0) + { + close(_wakeFd); + } _wakeFd = 0; if (_timerTs != null) { diff --git a/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs b/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs index 0713035a..2cd9cbed 100644 --- a/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs +++ b/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs @@ -348,6 +348,12 @@ private void OpenTcpListeners() } _listenFds = new int[1 + _tcp.ExtraPorts.Length]; + + // -1, not the default 0: OpenReusePortListener below can throw partway through, and + // Teardown runs on that path now. An unset slot left at 0 would have it close stdin, + // which hands the number 0 to the next socket the process opens - and the next teardown + // to close 0 then shuts a live connection belonging to somebody else. + Array.Fill(_listenFds, -1); _listenPorts = new ushort[_listenFds.Length]; _listenPorts[0] = _port; for (int i = 0; i < _tcp.ExtraPorts.Length; i++) @@ -364,7 +370,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 a8d1f5d4..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(); diff --git a/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs b/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs new file mode 100644 index 00000000..e33420ce --- /dev/null +++ b/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs @@ -0,0 +1,76 @@ +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 - and 0 is stdin, not something +/// this library ever opened. Closing it is not a leak but the opposite: the number goes back to +/// the process and the next socket opened takes it, after which the next teardown to close 0 +/// shuts a live connection belonging to somebody else. The same holds for _wakeFd, which +/// OpenWakeFd only sets near the end of setup. +/// +/// /proc/self/fd/0 is the cheapest way to ask whether fd 0 is still open, and it needs no +/// P/Invoke: the entry exists exactly while the descriptor 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 plain listener sets neither SO_REUSEADDR nor SO_REUSEPORT, and a SO_REUSEPORT bind + // is refused unless EVERY socket on the port asked for it. So the reactor's bind to + // this port fails with EADDRINUSE - deterministically, and without needing privilege + // or a race. That throw lands inside OpenTcpListeners, which runs before OpenWakeFd: + // both _listenFds[0] and _wakeFd are still 0 when the finally calls Teardown. + 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); From 910cf5eca298aba325bf874a941cd365cc63e152 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 10:38:03 +0100 Subject: [PATCH 07/16] reactor: take the wake fd away before closing it, and wait out its writers Teardown now runs on the fault path, so a reactor that never started looping gives its eventfd number back to the process. WakeFdWrite reads _wakeFd and writes with no guard at all, from any thread - so a writer that had already read the old number puts 8 bytes into whatever socket took it next. That is somebody else's connection, and 8 bytes of eventfd counter in a TLS stream ends the handshake. Before this the number was never released on that path - a faulted reactor leaked its eventfd - so the write landed harmlessly in an fd nobody could reach. Releasing it is right; releasing it under a writer is not. Teardown takes the fd with an Interlocked.Exchange, so no new writer can obtain it, waits for the writers holding it to leave, and only then closes. WakeFdWrite counts itself in and reads 0 to mean the reactor is gone. Both are off the reactor thread, next to a syscall, so the cost is not measurable; the per-request path is untouched. E2E 195 passed 0 failed, Tls 142 passed 0 failed, Unit 48 passed 0 failed. --- src/ioxide/Reactor/Loop/Reactor.Drainers.cs | 28 ++++++++++++++++++--- src/ioxide/Reactor/Reactor.Runner.cs | 23 ++++++++++------- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/ioxide/Reactor/Loop/Reactor.Drainers.cs b/src/ioxide/Reactor/Loop/Reactor.Drainers.cs index 7754c52f..3eb44b08 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,32 @@ public sealed unsafe partial class Reactor #region Wake + // Writers currently holding the eventfd's number. Off-reactor callers only - every caller + // takes a direct path on the reactor thread - so these two interlocked operations sit next to + // a syscall and cost nothing measurable. + private int _wakeUsers; + + // The gate is what makes Teardown safe on a reactor that FAULTED. Teardown gives the eventfd's + // number back to the process, and a writer that had already read the old number would + // otherwise put 8 bytes into whatever socket took it next - somebody else's connection, not a + // wake-up. Teardown takes the fd away first, waits for the writers holding it to leave, and + // only then closes. Reading 0 here means exactly that: the reactor is gone, nothing to wake. 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/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index 77a0740b..c7a3580d 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -116,18 +116,23 @@ private void Teardown() CloseUdpFds(); CloseAcceptedTcpSockets(); - // Guarded because OpenWakeFd runs late in setup and Teardown now also follows a throw - // from anything before it, where _wakeFd is still its default 0 - stdin, not an eventfd. + // Taken away before it is closed, and the writers still holding it are waited out. + // WakeFdWrite runs on any thread, and Teardown now also follows a fault - at any instant, + // while a host is still handing work in. Closing under a writer would hand the number back + // to the process and let that writer's 8 bytes land in whatever socket took it next. // - // Zeroed rather than merely closed: WakeFdWrite is called from any thread and guards only - // on _wakeFd > 0, so leaving the old number here writes into whatever fd reused it. - // Teardown used to follow an explicit Stop(); it can now also follow a fault, at any - // instant, while a host is still handing work in. - if (_wakeFd > 0) + // Reading 0 covers the other half: OpenWakeFd runs late in setup, so a throw from anything + // before it arrives here with the field still at its default - stdin, not an eventfd. + int wakeFd = Interlocked.Exchange(ref _wakeFd, 0); + if (wakeFd > 0) { - close(_wakeFd); + SpinWait spin = default; + while (Volatile.Read(ref _wakeUsers) != 0) + { + spin.SpinOnce(); + } + close(wakeFd); } - _wakeFd = 0; if (_timerTs != null) { NativeMemory.Free(_timerTs); From 2fd9b1a660e8a89f5e02a6f8aee317bb54882443 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 11:10:20 +0100 Subject: [PATCH 08/16] BISECT PROBE: setup back outside the try, to test the Tls regression Temporary. Reverts only the try-scope half of ac8e0a3 - a throw from setup skips Teardown again - and keeps OnFault, the register normalisation, the fd-0 guards and the wake-fd gate. If CI is green with this, the regression is the setup-failure teardown and nothing else in that commit. --- src/ioxide/Reactor/Reactor.Runner.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index c7a3580d..c3970887 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -25,14 +25,11 @@ public void Run() BindReactorThread(); _ring = Ring.Create(_ringEntries); - // The try opens HERE, not at the loop: setup is where the leak actually happens. OnStart is - // user code and the test harness deliberately throws from it, which leaked the ring fd, the - // listener and the eventfd - three descriptors and ~745 KiB of RLIMIT_MEMLOCK - on every - // failed start. Ring.Create itself is outside because there is nothing to tear down until - // it returns. - try - { - + // BISECT PROBE (temporary): setup is back OUTSIDE the try, so a throw from it skips + // Teardown exactly as it did before ac8e0a3. Everything else this branch added stays. If + // CI goes green with this, the Tls regression is the setup-failure teardown and nothing + // else in the commit. + // // Transports: TCP always; UDP sockets + the QUIC demux only when configured (no-ops otherwise). OpenTcpListeners(); OpenUdpSockets(); @@ -61,6 +58,8 @@ public void Run() StartTicker(); + try + { if (_incremental) LoopIncremental(); else LoopSharedRing(); } From 26d9fe445a6b5e5d2d4f3d2ca33ba0f0a402a826 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 11:22:22 +0100 Subject: [PATCH 09/16] reactor: unregister the provided-buffer rings before closing the ring Bisected on CI: with setup inside the try, the Tls suite fails "identity: a 40 KB distinguished name" three runs out of three; with setup back outside it, the whole run is green (Tls 157 passed, 0 failed). main is 5 for 5 green. So running Teardown after a setup failure is the trigger, and nothing else in ac8e0a3. The mechanism is the ordering here. Unregistering a provided-buffer ring is synchronous; closing the ring fd is not - io_uring_release schedules the context's teardown onto a workqueue and returns. Teardown then freed _bufRing and _bufSlab (and the UDP pair) straight after Dispose, handing pages the kernel still had registered back to the allocator, to be handed out again to whatever allocated next. The per-connection rings of the incremental path always unregistered explicitly; the shared and UDP ones leaned on the close. It stayed invisible because the test harness leaves its servers running for the whole suite, so Teardown essentially never ran mid-run - until setup failures started reaching it, ~25 times per Tls run. Restores the setup coverage and unregisters both rings while the ring fd is still open. E2E 195 passed, 0 failed. --- src/ioxide/Reactor/Reactor.Runner.cs | 41 +++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index c3970887..5c1ee346 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -25,11 +25,14 @@ public void Run() BindReactorThread(); _ring = Ring.Create(_ringEntries); - // BISECT PROBE (temporary): setup is back OUTSIDE the try, so a throw from it skips - // Teardown exactly as it did before ac8e0a3. Everything else this branch added stays. If - // CI goes green with this, the Tls regression is the setup-failure teardown and nothing - // else in the commit. - // + // The try opens HERE, not at the loop: setup is where the leak actually happens. OnStart is + // user code and the test harness deliberately throws from it, which leaked the ring fd, the + // listener and the eventfd - three descriptors and ~745 KiB of RLIMIT_MEMLOCK - on every + // failed start. Ring.Create itself is outside because there is nothing to tear down until + // it returns. + try + { + // Transports: TCP always; UDP sockets + the QUIC demux only when configured (no-ops otherwise). OpenTcpListeners(); OpenUdpSockets(); @@ -58,8 +61,6 @@ public void Run() StartTicker(); - try - { if (_incremental) LoopIncremental(); else LoopSharedRing(); } @@ -143,6 +144,14 @@ private void Teardown() _opTimespecs = null; _opTimespecCapacity = 0; } + // Before the ring fd goes, and this ordering is the whole point: unregistering is + // synchronous, closing is not. io_uring_release schedules the context's teardown onto a + // workqueue and returns, so after Dispose the kernel can still hold these rings registered + // - and the frees just below would hand their pages back to the allocator underneath it. + // The per-connection rings of the incremental path always did this (see + // TeardownConnectionBufRing); the shared and UDP ones leaned on the close instead. + UnregisterSharedBufRings(); + _ring.Dispose(); // Shared provided-buffer ring (incremental mode allocates per connection instead). @@ -159,6 +168,24 @@ private void Teardown() FreeUdpMemory(); } + // Both are no-ops unless the corresponding ring was actually registered: incremental mode + // leaves _bufRing null (it registers 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; From c28e6ec9aac4ccd0497f1e5804ea51c837cfc6c0 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 11:33:11 +0100 Subject: [PATCH 10/16] BISECT PROBE 2: on setup failure, leak the listener and ring fd, free everything Temporary. Splits the setup-failure teardown into its two halves - fd closes and memory frees - to see which one the Tls suite reacts to. --- src/ioxide/Reactor/Reactor.Runner.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index 5c1ee346..a149a668 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -61,6 +61,7 @@ public void Run() StartTicker(); + _loopEntered = true; if (_incremental) LoopIncremental(); else LoopSharedRing(); } @@ -109,9 +110,19 @@ private void AnnounceListening() // 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. + // BISECT PROBE (temporary): on the setup-failure path only, leak the listener and the ring fd + // - exactly what main leaks - while still doing every memory free. Green says an fd close is + // what breaks the Tls suite; red says a free is. + private bool _loopEntered; + private void Teardown() { - CloseTcpListeners(); + bool probeKeepFds = !_loopEntered; + + if (!probeKeepFds) + { + CloseTcpListeners(); + } TeardownQuic(); CloseUdpFds(); CloseAcceptedTcpSockets(); @@ -152,7 +163,10 @@ private void Teardown() // TeardownConnectionBufRing); the shared and UDP ones leaned on the close instead. UnregisterSharedBufRings(); - _ring.Dispose(); + if (!probeKeepFds) + { + _ring.Dispose(); + } // Shared provided-buffer ring (incremental mode allocates per connection instead). if (_bufRing != null) From 85838a00b81f4ef9f0b645abd78f6011d810f995 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 11:44:58 +0100 Subject: [PATCH 11/16] BISECT PROBE 3: close the listener on setup failure, keep the ring fd Temporary. Probe 2 narrowed it to an fd close rather than a free; this splits the listener close from the ring close. --- src/ioxide/Reactor/Reactor.Runner.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index a149a668..322eb7ae 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -110,19 +110,16 @@ private void AnnounceListening() // 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. - // BISECT PROBE (temporary): on the setup-failure path only, leak the listener and the ring fd - // - exactly what main leaks - while still doing every memory free. Green says an fd close is - // what breaks the Tls suite; red says a free is. + // BISECT PROBE 3 (temporary): probe 2 said an fd close is what breaks the Tls suite, not a + // free. This one closes the listener and keeps only the RING fd. Green says the ring close is + // the culprit; red says the listener close is. private bool _loopEntered; private void Teardown() { - bool probeKeepFds = !_loopEntered; + bool probeKeepRing = !_loopEntered; - if (!probeKeepFds) - { - CloseTcpListeners(); - } + CloseTcpListeners(); TeardownQuic(); CloseUdpFds(); CloseAcceptedTcpSockets(); @@ -163,7 +160,7 @@ private void Teardown() // TeardownConnectionBufRing); the shared and UDP ones leaned on the close instead. UnregisterSharedBufRings(); - if (!probeKeepFds) + if (!probeKeepRing) { _ring.Dispose(); } From 6a33276c0f0f7849a2f2fda918ed4fc6ab22b73b Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 11:56:53 +0100 Subject: [PATCH 12/16] BISECT PROBE 4: munmap both rings on setup failure, leak only the ring fd Temporary. Probe 3 pinned it to _ring.Dispose(); this splits its munmaps from its close(ring_fd). --- src/ioxide/Reactor/Reactor.Runner.cs | 11 ++++------- src/ioxide/io_uring/Ring.cs | 11 ++++++++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index 322eb7ae..18ef899d 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -110,9 +110,9 @@ private void AnnounceListening() // 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. - // BISECT PROBE 3 (temporary): probe 2 said an fd close is what breaks the Tls suite, not a - // free. This one closes the listener and keeps only the RING fd. Green says the ring close is - // the culprit; red says the listener close is. + // BISECT PROBE 4 (temporary): probe 3 pinned it to _ring.Dispose(). This one still munmaps + // both rings on the setup-failure path and leaks only the ring FD. Green says close(ring_fd) + // is the culprit; red says the munmaps are. private bool _loopEntered; private void Teardown() @@ -160,10 +160,7 @@ private void Teardown() // TeardownConnectionBufRing); the shared and UDP ones leaned on the close instead. UnregisterSharedBufRings(); - if (!probeKeepRing) - { - _ring.Dispose(); - } + _ring.Dispose(closeFd: !probeKeepRing); // Shared provided-buffer ring (incremental mode allocates per connection instead). if (_bufRing != null) diff --git a/src/ioxide/io_uring/Ring.cs b/src/ioxide/io_uring/Ring.cs index 45d23b2d..4deb5686 100644 --- a/src/ioxide/io_uring/Ring.cs +++ b/src/ioxide/io_uring/Ring.cs @@ -224,6 +224,15 @@ public bool TryGetCqe(out IoUringCqe cqe) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void CqAdvance(uint n) => Volatile.Write(ref *_cqHead, *_cqHead + n); + // BISECT PROBE 4 (temporary): closeFd == false does the munmaps and leaks only the ring fd. + public void Dispose(bool closeFd) + { + _probeCloseFd = closeFd; + Dispose(); + } + + private bool _probeCloseFd = true; + public void Dispose() { if (_ringPtr != null) @@ -236,7 +245,7 @@ public void Dispose() munmap(_sqePtr, _sqeSize); _sqePtr = null; } - if (_fd > 0) + if (_fd > 0 && _probeCloseFd) { close(_fd); _fd = 0; } From 9b6efb2f7c042c1e127ec067a5347ef6c49b1cee Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 12:08:54 +0100 Subject: [PATCH 13/16] BISECT PROBE 5: close the ring via dup2, keeping its fd number occupied Temporary. Probe 4 pinned it to close(ring_fd). dup2 closes the ring exactly as close() does but parks /dev/null on the number, so it is never handed out. Green says fd-number recycling is the damage; red says the kernel teardown is. --- src/ioxide/Native/Native.File.cs | 3 +++ src/ioxide/Reactor/Reactor.Runner.cs | 24 ++++++++++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/ioxide/Native/Native.File.cs b/src/ioxide/Native/Native.File.cs index ca07a22e..5c1ed0ce 100644 --- a/src/ioxide/Native/Native.File.cs +++ b/src/ioxide/Native/Native.File.cs @@ -14,6 +14,9 @@ public static unsafe partial class Native { [DllImport("libc", EntryPoint = "open", SetLastError = true)] public static extern int open([MarshalAs(UnmanagedType.LPUTF8Str)] string path, int flags, int mode); + // BISECT PROBE 5 (temporary). + [DllImport("libc", SetLastError = true)] public static extern int dup2(int oldfd, int newfd); + [DllImport("libc", SetLastError = true)] public static extern long lseek(int fd, long offset, int whence); diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index 18ef899d..9dcc1147 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -110,9 +110,10 @@ private void AnnounceListening() // 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. - // BISECT PROBE 4 (temporary): probe 3 pinned it to _ring.Dispose(). This one still munmaps - // both rings on the setup-failure path and leaks only the ring FD. Green says close(ring_fd) - // is the culprit; red says the munmaps are. + // BISECT PROBE 5 (temporary): probe 4 pinned it to close(ring_fd) alone. This one still CLOSES + // the ring - dup2 over the number closes it exactly as close() would - but parks /dev/null on + // the number so it is never handed out again. Green says the damage is fd-number recycling; + // red says it is the kernel's ring teardown itself. private bool _loopEntered; private void Teardown() @@ -160,7 +161,22 @@ private void Teardown() // TeardownConnectionBufRing); the shared and UDP ones leaned on the close instead. UnregisterSharedBufRings(); - _ring.Dispose(closeFd: !probeKeepRing); + if (probeKeepRing) + { + int ringFd = _ring.Fd; + _ring.Dispose(closeFd: false); // munmaps only + + int devnull = open("/dev/null", O_RDONLY, 0); + if (devnull >= 0) + { + dup2(devnull, ringFd); // closes the ring AND keeps the number occupied + close(devnull); + } + } + else + { + _ring.Dispose(); + } // Shared provided-buffer ring (incremental mode allocates per connection instead). if (_bufRing != null) From bfe1802fda1af6ef1786bd59febf2d0d9d7eac8a Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 12:21:15 +0100 Subject: [PATCH 14/16] reactor: keep the ring fd when the start failed, and drop the bisect probes Final shape of the setup-path teardown. Bisected on CI, one variable per run: setup inside the try, everything torn down Tls FAILS (3 of 3) setup outside the try (main's behaviour) green ... teardown minus the fd closes green ... minus the ring close only green ... ring closed by dup2, number kept green main, same commit, reruns green (5 of 5) The munmaps, the listener close, the eventfd close and every free are all harmless. Closing the ring DESCRIPTOR is not - and dup2'ing /dev/null over the number tears the ring down exactly as close() does while never handing the number back, which is green. So the damage is fd-number recycling: something in this library acts on an fd number it no longer owns, and nothing frees a low number mid-run today, which is why main never shows it. That is a real defect and it is not this PR's. Filed separately rather than buried here; closing the fd on this path only made it reachable. So the ring fd is kept when the start failed, which is what main already does - nothing regresses - while the listener, the eventfd, both mappings and every allocation are now released, which main does not. E2E 195 passed, 0 failed. --- src/ioxide/Native/Native.File.cs | 3 -- src/ioxide/Reactor/Reactor.Runner.cs | 44 +++++++++++++--------------- src/ioxide/io_uring/Ring.cs | 12 +++++--- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/ioxide/Native/Native.File.cs b/src/ioxide/Native/Native.File.cs index 5c1ed0ce..ca07a22e 100644 --- a/src/ioxide/Native/Native.File.cs +++ b/src/ioxide/Native/Native.File.cs @@ -14,9 +14,6 @@ public static unsafe partial class Native { [DllImport("libc", EntryPoint = "open", SetLastError = true)] public static extern int open([MarshalAs(UnmanagedType.LPUTF8Str)] string path, int flags, int mode); - // BISECT PROBE 5 (temporary). - [DllImport("libc", SetLastError = true)] public static extern int dup2(int oldfd, int newfd); - [DllImport("libc", SetLastError = true)] public static extern long lseek(int fd, long offset, int whence); diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index 9dcc1147..faf60c1a 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -61,7 +61,7 @@ public void Run() StartTicker(); - _loopEntered = true; + _ranLoop = true; if (_incremental) LoopIncremental(); else LoopSharedRing(); } @@ -110,15 +110,14 @@ private void AnnounceListening() // 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. - // BISECT PROBE 5 (temporary): probe 4 pinned it to close(ring_fd) alone. This one still CLOSES - // the ring - dup2 over the number closes it exactly as close() would - but parks /dev/null on - // the number so it is never handed out again. Green says the damage is fd-number recycling; - // red says it is the kernel's ring teardown itself. - private bool _loopEntered; + /// Set once the loop is entered, so Teardown can tell a failed start from a stop. + private bool _ranLoop; private void Teardown() { - bool probeKeepRing = !_loopEntered; + // Everything below runs either way. Only the ring DESCRIPTOR is treated differently, and + // only when setup failed - see where it is closed. + bool startFailed = !_ranLoop; CloseTcpListeners(); TeardownQuic(); @@ -161,22 +160,21 @@ private void Teardown() // TeardownConnectionBufRing); the shared and UDP ones leaned on the close instead. UnregisterSharedBufRings(); - if (probeKeepRing) - { - int ringFd = _ring.Fd; - _ring.Dispose(closeFd: false); // munmaps only - - int devnull = open("/dev/null", O_RDONLY, 0); - if (devnull >= 0) - { - dup2(devnull, ringFd); // closes the ring AND keeps the number occupied - close(devnull); - } - } - else - { - _ring.Dispose(); - } + // Both mappings go back either way; the descriptor is kept when the start failed. + // + // Bisected on CI, one variable per run: with the ring fd closed here the Tls suite loses + // "identity: a 40 KB distinguished name" three runs out of three, and main is green five + // for five. Keeping the fd is green; so is closing it by dup2'ing /dev/null over the + // number, which tears the ring down exactly as close() does but never hands the NUMBER + // back. So the damage is fd-number recycling, not the ring teardown: something in this + // library acts on an fd number it no longer owns, and nothing frees a low number mid-run + // today, which is why main never shows it. Tracked separately - it is not this PR's bug, + // and closing the fd here only made it reachable. + // + // The cost of keeping it is the ring's RLIMIT_MEMLOCK charge, held until the process + // exits. That is what main already does on this path, so nothing regresses; the listener, + // the eventfd, both mappings and every allocation above are released, which main does not. + _ring.Dispose(closeFd: !startFailed); // Shared provided-buffer ring (incremental mode allocates per connection instead). if (_bufRing != null) diff --git a/src/ioxide/io_uring/Ring.cs b/src/ioxide/io_uring/Ring.cs index 4deb5686..ac839e2d 100644 --- a/src/ioxide/io_uring/Ring.cs +++ b/src/ioxide/io_uring/Ring.cs @@ -224,14 +224,18 @@ public bool TryGetCqe(out IoUringCqe cqe) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void CqAdvance(uint n) => Volatile.Write(ref *_cqHead, *_cqHead + n); - // BISECT PROBE 4 (temporary): closeFd == false does the munmaps and leaks only the ring fd. + /// + /// Unmaps both rings and, unless says otherwise, closes the ring + /// descriptor. Keeping it is for one case only - see Reactor.Teardown - and costs the ring's + /// RLIMIT_MEMLOCK charge until the process exits. + /// public void Dispose(bool closeFd) { - _probeCloseFd = closeFd; + _closeFd = closeFd; Dispose(); } - private bool _probeCloseFd = true; + private bool _closeFd = true; public void Dispose() { @@ -245,7 +249,7 @@ public void Dispose() munmap(_sqePtr, _sqeSize); _sqePtr = null; } - if (_fd > 0 && _probeCloseFd) + if (_fd > 0 && _closeFd) { close(_fd); _fd = 0; } From 648db40ffd880661a7d5816f2c9e189c35a8ea56 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 12:30:09 +0100 Subject: [PATCH 15/16] reactor: point the kept-ring-fd note at #242 --- src/ioxide/Reactor/Reactor.Runner.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index faf60c1a..28be4a55 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -168,8 +168,8 @@ private void Teardown() // number, which tears the ring down exactly as close() does but never hands the NUMBER // back. So the damage is fd-number recycling, not the ring teardown: something in this // library acts on an fd number it no longer owns, and nothing frees a low number mid-run - // today, which is why main never shows it. Tracked separately - it is not this PR's bug, - // and closing the fd here only made it reachable. + // today, which is why main never shows it. That is #242 - not this PR's bug; closing the + // fd here only made it reachable. // // The cost of keeping it is the ring's RLIMIT_MEMLOCK charge, held until the process // exits. That is what main already does on this path, so nothing regresses; the listener, From d72e2a39af0437c362d23ccae80e7eb4fa62e600 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 21 Sep 2026 17:14:54 +0100 Subject: [PATCH 16/16] trim the comments added by this PR 196 comment lines down to 132, keeping every why and dropping the retelling. Also moves the _ranLoop field above Teardown's header comment, which it had been wedged into. --- src/ioxide/Native/Native.IoUring.cs | 14 ++-- src/ioxide/Reactor/Loop/Reactor.Drainers.cs | 12 ++-- .../Reactor/Loop/Reactor.Loop.Incremental.cs | 17 ++--- .../Reactor/Loop/Reactor.Loop.SharedRing.cs | 19 +++--- src/ioxide/Reactor/Reactor.RingHost.cs | 8 +-- src/ioxide/Reactor/Reactor.Runner.cs | 65 ++++++------------- .../Reactor/Transport/Tcp/Reactor.Tcp.cs | 7 +- src/ioxide/io_uring/Ring.cs | 29 ++++----- .../IoxideConnectionListener.cs | 5 +- .../Core/ReactorSetupTeardownTests.cs | 21 +++--- tests/Ioxide.Tests.Harness/TestServer.cs | 13 ++-- tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs | 32 +++------ 12 files changed, 87 insertions(+), 155 deletions(-) diff --git a/src/ioxide/Native/Native.IoUring.cs b/src/ioxide/Native/Native.IoUring.cs index ff3555ff..a2c01ed7 100644 --- a/src/ioxide/Native/Native.IoUring.cs +++ b/src/ioxide/Native/Native.IoUring.cs @@ -76,12 +76,11 @@ public static unsafe partial class Native { public const int MAP_SHARED = 1; public const int MAP_POPULATE = 0x8000; - // All three go through glibc's syscall(), which reports failure as -1 with the code in errno - - // it never returns -errno. So every one of them needs SetLastError, and the wrappers below - // normalise to liburing's convention: a negative errno, which is what every caller compares - // against. Two of these were declared without it (#220), which made three branches dead code: - // Ring.Create's fallback for kernels without IORING_SETUP_NO_SQARRAY, and the EINTR/EAGAIN/EBUSY - // tolerance in both reactor loops - so one signal delivered to a reactor thread ended it. + // 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); @@ -91,8 +90,7 @@ public static unsafe partial class Native { [DllImport("libc", EntryPoint = "syscall", SetLastError = true)] private static extern long syscall4(long nr, uint a1, uint a2, void* a3, uint a4); - // Test the long before narrowing: a successful io_uring_enter returns a submission count, and - // errno is only meaningful on the failure branch. + // 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); diff --git a/src/ioxide/Reactor/Loop/Reactor.Drainers.cs b/src/ioxide/Reactor/Loop/Reactor.Drainers.cs index 3eb44b08..a13fc3c0 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Drainers.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Drainers.cs @@ -16,16 +16,12 @@ public sealed unsafe partial class Reactor #region Wake - // Writers currently holding the eventfd's number. Off-reactor callers only - every caller - // takes a direct path on the reactor thread - so these two interlocked operations sit next to - // a syscall and cost nothing measurable. + // 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; - // The gate is what makes Teardown safe on a reactor that FAULTED. Teardown gives the eventfd's - // number back to the process, and a writer that had already read the old number would - // otherwise put 8 bytes into whatever socket took it next - somebody else's connection, not a - // wake-up. Teardown takes the fd away first, waits for the writers holding it to leave, and - // only then closes. Reading 0 here means exactly that: the reactor is gone, nothing to wake. private void WakeFdWrite() { Interlocked.Increment(ref _wakeUsers); diff --git a/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs b/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs index 45bbccda..ca734f21 100644 --- a/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs +++ b/src/ioxide/Reactor/Loop/Reactor.Loop.Incremental.cs @@ -177,17 +177,14 @@ private void LoopIncremental() RearmStarvedRecvs(); QuicFireDueTimers(); - // These three are the transient ones and the loop simply carries on: EINTR is a signal, - // EAGAIN is the kernel short of resources, EBUSY means overflow entries could not be - // flushed - and the fall-through below drains the CQ, which is exactly what EBUSY wants. - // Until #220 this comparison could never match, because the wrapper returned -1 for - // every failure, so the first signal delivered to a reactor thread ended it. + // 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 (EBADF, EINVAL, ENXIO on a dying - // ring). Throwing rather than breaking, because a reactor that vanishes while the - // process keeps reporting healthy is the worst of both. Run's finally tears the ring - // down; whether the process then dies is the host's decision, via Reactor.OnFault - - // on a bare Thread with no handler, an unhandled exception ends the process. + // 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) { diff --git a/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs b/src/ioxide/Reactor/Loop/Reactor.Loop.SharedRing.cs index 9a24ed75..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; @@ -54,17 +54,14 @@ private void LoopSharedRing() RearmStarvedRecvs(); QuicFireDueTimers(); - // These three are the transient ones and the loop simply carries on: EINTR is a signal, - // EAGAIN is the kernel short of resources, EBUSY means overflow entries could not be - // flushed - and the fall-through below drains the CQ, which is exactly what EBUSY wants. - // Until #220 this comparison could never match, because the wrapper returned -1 for - // every failure, so the first signal delivered to a reactor thread ended it. + // 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 (EBADF, EINVAL, ENXIO on a dying - // ring). Throwing rather than breaking, because a reactor that vanishes while the - // process keeps reporting healthy is the worst of both. Run's finally tears the ring - // down; whether the process then dies is the host's decision, via Reactor.OnFault - - // on a bare Thread with no handler, an unhandled exception ends the process. + // 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) { diff --git a/src/ioxide/Reactor/Reactor.RingHost.cs b/src/ioxide/Reactor/Reactor.RingHost.cs index 558ef57b..140ea762 100644 --- a/src/ioxide/Reactor/Reactor.RingHost.cs +++ b/src/ioxide/Reactor/Reactor.RingHost.cs @@ -63,11 +63,9 @@ public T GetService() where T : class /// the process down deliberately. /// /// - /// Without a handler the exception propagates out of . On a bare - /// new Thread(reactor.Run) - which is how every sample and ioxide.Kestrel start - /// one - that terminates the process. Which of the two is right is the host's call, not this - /// library's: losing one shard of N silently is indefensible, and so is taking the whole server - /// down without being asked. + /// 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; diff --git a/src/ioxide/Reactor/Reactor.Runner.cs b/src/ioxide/Reactor/Reactor.Runner.cs index 28be4a55..395ebd78 100644 --- a/src/ioxide/Reactor/Reactor.Runner.cs +++ b/src/ioxide/Reactor/Reactor.Runner.cs @@ -25,11 +25,9 @@ public void Run() BindReactorThread(); _ring = Ring.Create(_ringEntries); - // The try opens HERE, not at the loop: setup is where the leak actually happens. OnStart is - // user code and the test harness deliberately throws from it, which leaked the ring fd, the - // listener and the eventfd - three descriptors and ~745 KiB of RLIMIT_MEMLOCK - on every - // failed start. Ring.Create itself is outside because there is nothing to tear down until - // it returns. + // 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 { @@ -72,10 +70,7 @@ public void Run() } finally { - // Whatever happened - a fatal io_uring_enter, a throw from OnStart, GetSqeOrFlush - // giving up on a full SQ - the ring fd, both mmaps, the eventfd and the buffer slab go - // back. Ring memory is charged against RLIMIT_MEMLOCK, so leaking rings is how a - // long-lived host eventually cannot create one. + // Runs on every exit path: a fatal io_uring_enter, a throw from OnStart, a full SQ. Teardown(); } } @@ -106,17 +101,14 @@ 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() { - // Everything below runs either way. Only the ring DESCRIPTOR is treated differently, and - // only when setup failed - see where it is closed. bool startFailed = !_ranLoop; CloseTcpListeners(); @@ -124,13 +116,9 @@ private void Teardown() CloseUdpFds(); CloseAcceptedTcpSockets(); - // Taken away before it is closed, and the writers still holding it are waited out. - // WakeFdWrite runs on any thread, and Teardown now also follows a fault - at any instant, - // while a host is still handing work in. Closing under a writer would hand the number back - // to the process and let that writer's 8 bytes land in whatever socket took it next. - // - // Reading 0 covers the other half: OpenWakeFd runs late in setup, so a throw from anything - // before it arrives here with the field still at its default - stdin, not an eventfd. + // 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) { @@ -152,28 +140,16 @@ private void Teardown() _opTimespecs = null; _opTimespecCapacity = 0; } - // Before the ring fd goes, and this ordering is the whole point: unregistering is - // synchronous, closing is not. io_uring_release schedules the context's teardown onto a - // workqueue and returns, so after Dispose the kernel can still hold these rings registered - // - and the frees just below would hand their pages back to the allocator underneath it. - // The per-connection rings of the incremental path always did this (see - // TeardownConnectionBufRing); the shared and UDP ones leaned on the close instead. + // 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. - // - // Bisected on CI, one variable per run: with the ring fd closed here the Tls suite loses - // "identity: a 40 KB distinguished name" three runs out of three, and main is green five - // for five. Keeping the fd is green; so is closing it by dup2'ing /dev/null over the - // number, which tears the ring down exactly as close() does but never hands the NUMBER - // back. So the damage is fd-number recycling, not the ring teardown: something in this - // library acts on an fd number it no longer owns, and nothing frees a low number mid-run - // today, which is why main never shows it. That is #242 - not this PR's bug; closing the - // fd here only made it reachable. - // - // The cost of keeping it is the ring's RLIMIT_MEMLOCK charge, held until the process - // exits. That is what main already does on this path, so nothing regresses; the listener, - // the eventfd, both mappings and every allocation above are released, which main does not. + // 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). @@ -190,9 +166,8 @@ private void Teardown() FreeUdpMemory(); } - // Both are no-ops unless the corresponding ring was actually registered: incremental mode - // leaves _bufRing null (it registers one ring per connection instead), and a reactor with no - // datagram transport leaves _udpBufRing null. + // 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) diff --git a/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs b/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs index 2cd9cbed..1d310234 100644 --- a/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs +++ b/src/ioxide/Reactor/Transport/Tcp/Reactor.Tcp.cs @@ -349,10 +349,9 @@ private void OpenTcpListeners() _listenFds = new int[1 + _tcp.ExtraPorts.Length]; - // -1, not the default 0: OpenReusePortListener below can throw partway through, and - // Teardown runs on that path now. An unset slot left at 0 would have it close stdin, - // which hands the number 0 to the next socket the process opens - and the next teardown - // to close 0 then shuts a live connection belonging to somebody else. + // -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; diff --git a/src/ioxide/io_uring/Ring.cs b/src/ioxide/io_uring/Ring.cs index ac839e2d..cc296b87 100644 --- a/src/ioxide/io_uring/Ring.cs +++ b/src/ioxide/io_uring/Ring.cs @@ -37,25 +37,19 @@ public sealed unsafe class Ring : IDisposable /// 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 that creates and drops reactors faster than the kernel reclaims them - a test - /// suite standing servers up and tearing them down is the usual shape - gets ENOMEM while - /// nothing is actually leaking. Measured: creating and immediately closing 30,000 rings failed - /// 14,788 times, and every failure cleared on a retry 5 ms later. + /// 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. /// - /// 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: with - /// an 8 MB RLIMIT_MEMLOCK and the default ring size, measured, a third of attempts still fail - /// first time and a few per thousand exhaust all six. There the answer is a bigger limit or a - /// smaller ring, and the message below says so. + /// 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 - about 155ms in total. Sized from the measured reclaim latency of a - // SINGLE ring, whose median is ~20ms and whose tail reaches 47ms under load. A shorter - // schedule looked sufficient against a BURST, where many rings are in flight and one is - // always coming back within a few ms, but the one-at-a-time case a restarting server - // produces is far slower and a 30ms budget still failed a fifth of the time. + // 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); @@ -225,9 +219,8 @@ public bool TryGetCqe(out IoUringCqe cqe) public void CqAdvance(uint n) => Volatile.Write(ref *_cqHead, *_cqHead + n); /// - /// Unmaps both rings and, unless says otherwise, closes the ring - /// descriptor. Keeping it is for one case only - see Reactor.Teardown - and costs the ring's - /// RLIMIT_MEMLOCK charge until the process exits. + /// 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) { diff --git a/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs b/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs index 83e5fbd1..96175f98 100644 --- a/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs +++ b/src/serving/ioxide.Kestrel/IoxideConnectionListener.cs @@ -77,9 +77,8 @@ public IoxideConnectionListener(IPEndPoint endpoint, IoxideTransportOptions opti } onReactorStart?.Invoke(r); }; - // A reactor is one shard of N. Losing one should not end the process, but it must not - // be silent either - without this, a fatal io_uring errno propagates off a bare Thread - // and aborts the host. + // 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}"); diff --git a/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs b/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs index e33420ce..9a5bc8e3 100644 --- a/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs +++ b/tests/Ioxide.Tests.E2E/Core/ReactorSetupTeardownTests.cs @@ -9,14 +9,12 @@ namespace Ioxide.Tests; /// 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 - and 0 is stdin, not something -/// this library ever opened. Closing it is not a leak but the opposite: the number goes back to -/// the process and the next socket opened takes it, after which the next teardown to close 0 -/// shuts a live connection belonging to somebody else. The same holds for _wakeFd, which -/// OpenWakeFd only sets near the end of setup. +/// 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 is the cheapest way to ask whether fd 0 is still open, and it needs no -/// P/Invoke: the entry exists exactly while the descriptor does. +/// /proc/self/fd/0 answers whether fd 0 is open without a P/Invoke: it exists while it does. /// internal static class ReactorSetupTeardownTests { @@ -26,11 +24,10 @@ public static void Register(Runner runner) { Assert.True(File.Exists("/proc/self/fd/0"), "fd 0 must be open before the test says anything"); - // A plain listener sets neither SO_REUSEADDR nor SO_REUSEPORT, and a SO_REUSEPORT bind - // is refused unless EVERY socket on the port asked for it. So the reactor's bind to - // this port fails with EADDRINUSE - deterministically, and without needing privilege - // or a race. That throw lands inside OpenTcpListeners, which runs before OpenWakeFd: - // both _listenFds[0] and _wakeFd are still 0 when the finally calls Teardown. + // 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; diff --git a/tests/Ioxide.Tests.Harness/TestServer.cs b/tests/Ioxide.Tests.Harness/TestServer.cs index 9ea175f0..541e03c5 100644 --- a/tests/Ioxide.Tests.Harness/TestServer.cs +++ b/tests/Ioxide.Tests.Harness/TestServer.cs @@ -632,14 +632,11 @@ public static int StartQuicServingH3Driver( /// as unowned. /// /// - /// Ordering, not speed, is what this fixes. A reactor whose OnStart throws faults `started` - /// immediately but is only recorded in StartupFailures once Run has fully unwound - and Run now - /// tears the ring down on the way out, which takes milliseconds. So the consumer below can look - /// before the producer writes, remove nothing, and the entry then lands with nobody left to - /// claim it. Marking the port closes the race whichever way round it goes. - /// - /// Ports are never reused within a run (ReserveFreePort only moves forward), so the port is a - /// sound key for one server's lifetime. + /// 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(); diff --git a/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs b/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs index 98e86f89..40b638a4 100644 --- a/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs +++ b/tests/Ioxide.Tests.Unit/SyscallErrnoTests.cs @@ -7,26 +7,13 @@ namespace Ioxide.Tests; /// as a negative errno, liburing's convention (#220). /// /// -/// glibc's syscall() returns -1 and puts the code in errno; it never returns -errno. An -/// import declared without SetLastError therefore loses the code entirely, and every -/// comparison against a specific errno becomes dead: +/// 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. /// -/// -/// Ring.cs:46 if (fd == -EINVAL) // the pre-6.6 fallback -/// Reactor.Loop.SharedRing.cs:60 rc != -EINTR && rc != -EAGAIN && rc != -EBUSY -/// Reactor.Loop.Incremental.cs:181 rc != -EINTR && rc != -EAGAIN && rc != -EBUSY -/// -/// -/// So NO_SQARRAY never falls back on 6.1-6.5, and one interrupted io_uring_enter - a SIGHUP -/// handler, a profiler's SIGPROF, anything the kernel routes to a reactor thread - ends that -/// reactor, while the process carries on reporting healthy at reduced capacity. -/// -/// The three declarations sat next to each other in Native.IoUring.cs and only one of them, -/// syscall4 for io_uring_register, was declared SetLastError = true - which is how the omission -/// went unnoticed. All three now normalise to a negative errno. -/// -/// No sockets, no signals, no timing here: ask the kernel for something it must refuse and read -/// what comes back. +/// 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 { @@ -40,8 +27,7 @@ public static void Register(Runner runner) { runner.Test("syscall: io_uring_setup reports a negative errno, not -1", () => { - // entries == 0 is refused by every kernel, so this needs no feature detection and - // cannot pass by accident on a machine that happens to support something. + // 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}"); @@ -50,8 +36,8 @@ public static void Register(Runner runner) runner.Test("syscall: io_uring_enter reports a negative errno, not -1", () => { - // -1 is not a ring descriptor, so the kernel answers EBADF. This is the value the two - // loops compare against -EINTR/-EAGAIN/-EBUSY to decide whether to keep going. + // 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}");