From e9ff089a589a49389f555aebdc4609e8911b2080 Mon Sep 17 00:00:00 2001 From: Rafal Maciag Date: Sun, 18 Oct 2026 17:37:39 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(testing):=20the=20port-lock=20directory?= =?UTF-8?q?=20works=20for=20every=20user=20of=20a=20shared=20box=20?= =?UTF-8?q?=E2=80=94=20created=201777=20(sticky)=20with=200666=20lock=20fi?= =?UTF-8?q?les=20whatever=20the=20umask;=20a=20lock=20file=20this=20user?= =?UTF-8?q?=20may=20not=20open=20is=20a=20port=20to=20skip;=20a=20director?= =?UTF-8?q?y=20in=20which=20nothing=20can=20be=20locked=20is=20refused,=20?= =?UTF-8?q?named,=20with=20the=20chmod=20that=20fixes=20it=20=E2=80=94=20n?= =?UTF-8?q?ever=20an=20unhandled=20throw=20or=20an=20endless=20search;=20a?= =?UTF-8?q?=20failed=20start=20whose=20cleanup=20also=20failed=20throws=20?= =?UTF-8?q?an=20AggregateException=20(start=20failure=20first)=20so=20both?= =?UTF-8?q?=20are=20printed;=20the=20race=20row=20asserts=20two=20results?= =?UTF-8?q?=20before=20its=20only/unique=20claims=20(rev-reach=209de418b9d?= =?UTF-8?q?=20m1,=20m2,=20note)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The directory is chmodded only when THIS process creates it: an existing one is used as it is (another user's cannot be chmodded — and an own 0555 stand-in, chmodded, made the refusal row pass vacuously: caught by the row going red first). Rows: modes 1777/0666; an unopenable lock file skipped; a 0555 directory refused named; both failures in ToString. Mutants red: the UnauthorizedAccessException catch removed (2 rows); no chmod; the cleanup failure back in Data; the race row with one child (only HaveCount(2) catches it). Co-Authored-By: Claude Opus 5.5 --- src/MicroPlumberd.Testing/EventStoreServer.cs | 7 +- src/MicroPlumberd.Testing/PortLock.cs | 70 +++++++++++++++---- .../EventStoreServerStartFailureTests.cs | 12 ++-- .../Integration/StorePortRaceTests.cs | 1 + src/MicroPlumberd.Tests/Unit/PortLockTests.cs | 56 +++++++++++++++ 5 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 src/MicroPlumberd.Tests/Unit/PortLockTests.cs diff --git a/src/MicroPlumberd.Testing/EventStoreServer.cs b/src/MicroPlumberd.Testing/EventStoreServer.cs index 762dd4d..081f6f8 100644 --- a/src/MicroPlumberd.Testing/EventStoreServer.cs +++ b/src/MicroPlumberd.Testing/EventStoreServer.cs @@ -220,8 +220,13 @@ private async Task StartAndWait(bool wait, bool inMemory, Time { // The START failure is the one to see (dev-clean, 2026-10-18): a cleanup that throws too (Docker 409 "removal // already in progress") replaced it, and the first failure was lost. Cleanup's failure rides along, never instead. + // Both visible (rev-reach 9de418b9d m2): Data is printed by neither xUnit nor ToString. The start failure FIRST. try { await Cleanup(); } - catch (Exception cleanup) { started.Data["MicroPlumberd.Testing.CleanupFailure"] = cleanup.ToString(); } + catch (Exception cleanup) + { + throw new AggregateException($"Store '{ContainerName}' failed to start, and removing its container failed too.", + started, cleanup); + } System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(started).Throw(); throw; // unreachable } diff --git a/src/MicroPlumberd.Testing/PortLock.cs b/src/MicroPlumberd.Testing/PortLock.cs index 8fa978a..d0a20f5 100644 --- a/src/MicroPlumberd.Testing/PortLock.cs +++ b/src/MicroPlumberd.Testing/PortLock.cs @@ -1,6 +1,3 @@ -using System.Net; -using System.Net.Sockets; - namespace MicroPlumberd.Testing; /// @@ -8,41 +5,90 @@ namespace MicroPlumberd.Testing; /// PortSearcher starts at the same port, and a free-port probe only sees a port as taken once its container has STARTED — /// seconds later — so two processes chose the same port. A lock file per port, opened exclusively (on Unix .NET takes an /// advisory flock for FileShare.None), is held until the store is disposed and released by the OS if the process dies. +/// +/// The directory is shared by every USER of the machine (rev-reach 9de418b9d m1): it is made 1777 (sticky) and each lock +/// file 0666, after creation, whatever the creator's umask. A lock file this user cannot open (another user's, made before +/// these modes) is a port to skip; a directory in which nothing can be locked is refused, named — never an unhandled throw +/// and never an endless search. +/// /// internal sealed class PortLock : IDisposable { /// One directory for every process on the machine (not the per-user/per-process TMPDIR). - internal static string Directory { get; set; } = OperatingSystem.IsWindows() + internal static readonly string DefaultDirectory = OperatingSystem.IsWindows() ? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), "microplumberd-testing-ports") : "/tmp/microplumberd-testing-ports"; + /// Consecutive ports this user may not lock before the directory itself is judged unusable. + internal const int DeniedLimit = 20; + + private const UnixFileMode SharedDirectory = (UnixFileMode)0b1_111_111_111; // 1777: sticky, rwx for all + private const UnixFileMode SharedFile = (UnixFileMode)0b110_110_110; // 0666 + private readonly FileStream _held; private PortLock(FileStream held) => _held = held; /// The next port that no other process holds AND that is free to bind, with its lock held. - internal static (int Port, PortLock Lock) Acquire(PortSearcher searcher) + internal static (int Port, PortLock Lock) Acquire(PortSearcher searcher, string? directory = null) { - System.IO.Directory.CreateDirectory(Directory); + directory ??= DefaultDirectory; + // Shared modes on the directory only when THIS process creates it: an existing one is used as it is — another + // user's cannot be chmodded, and one we cannot use is refused below, named, with the chmod that fixes it. + if (!Directory.Exists(directory)) + { + Directory.CreateDirectory(directory); + TryChmod(() => File.SetUnixFileMode(directory, SharedDirectory)); + } + + var denied = 0; while (true) { var port = searcher.FindNextAvailablePort(); - var held = TryHold(port); - if (held != null) return (port, held); + switch (TryHold(directory, port)) + { + case Held { Lock: var held }: return (port, held); + case Denied when ++denied >= DeniedLimit: + throw new InvalidOperationException( + $"No store port could be locked in '{directory}': this user may not create or open lock files there " + + $"({DeniedLimit} ports in a row). Make it shared — chmod 1777 '{directory}' — or remove it so it is recreated."); + case Denied: continue; + default: denied = 0; continue; // held by another process: the next port + } } } - internal static PortLock? TryHold(int port) + internal abstract record Outcome; + internal sealed record Held(PortLock Lock) : Outcome; + internal sealed record Taken : Outcome; + internal sealed record Denied : Outcome; + + internal static Outcome TryHold(string directory, int port) { + var path = Path.Combine(directory, $"{port}.lock"); try { - var stream = new FileStream(Path.Combine(Directory, $"{port}.lock"), FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); - return new PortLock(stream); + var stream = new FileStream(path, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + TryChmod(() => File.SetUnixFileMode(path, SharedFile)); + return new Held(new PortLock(stream)); + } + catch (UnauthorizedAccessException) + { + return new Denied(); // another user's lock file, or a directory this user may not write } catch (IOException) { - return null; // another process holds this port + return new Taken(); // another process holds this port } } + /// Best effort: a path another user owns cannot be chmodded by this one — it is then used as it is. + private static void TryChmod(Action chmod) + { + if (OperatingSystem.IsWindows()) return; + try { chmod(); } + catch (UnauthorizedAccessException) { } + catch (IOException) { } + } + public void Dispose() => _held.Dispose(); } diff --git a/src/MicroPlumberd.Tests/Integration/EventStoreServerStartFailureTests.cs b/src/MicroPlumberd.Tests/Integration/EventStoreServerStartFailureTests.cs index 3dc7355..f3dcda7 100644 --- a/src/MicroPlumberd.Tests/Integration/EventStoreServerStartFailureTests.cs +++ b/src/MicroPlumberd.Tests/Integration/EventStoreServerStartFailureTests.cs @@ -7,7 +7,8 @@ namespace MicroPlumberd.Tests.Integration; /// /// A failed start removes its container — and if that removal ALSO fails (two processes removing one container gave Docker /// 409 "removal already in progress", 2026-10-18), the START failure is still the one thrown; the cleanup failure rides -/// along in Data. It used to replace it, and the first failure was lost (dev-clean's observation). +/// along, both printed (an AggregateException, start failure first). It used to replace it, and the first failure was lost +/// (dev-clean's observation); then it rode in Data, which nothing prints (rev-reach 9de418b9d m2). /// [TestCategory("Integration")] public class EventStoreServerStartFailureTests @@ -32,9 +33,12 @@ public async Task AStartThatFails_AndWhoseCleanupAlsoFails_ThrowsTheStartFailure { var thrown = await Record.ExceptionAsync(() => server.StartInDocker()); - thrown.Should().BeOfType("the start failure is the first failure, and it is the one thrown"); - thrown!.Data["MicroPlumberd.Testing.CleanupFailure"].Should().BeOfType() - .Which.Should().Contain("already in progress", "the cleanup failure rides along, not instead"); + var both = thrown.Should().BeOfType().Subject; + both.InnerExceptions.Should().HaveCount(2); + both.InnerExceptions[0].Should().BeOfType("the start failure is the first failure, and it comes first"); + both.InnerExceptions[1].Message.Should().Contain("already in progress", "the cleanup failure rides along, not instead"); + both.ToString().Should().Contain("did not become ready").And.Contain("already in progress", + "both are printed — Data was printed by neither xUnit nor ToString"); } finally { diff --git a/src/MicroPlumberd.Tests/Integration/StorePortRaceTests.cs b/src/MicroPlumberd.Tests/Integration/StorePortRaceTests.cs index 2041b27..53cd9f7 100644 --- a/src/MicroPlumberd.Tests/Integration/StorePortRaceTests.cs +++ b/src/MicroPlumberd.Tests/Integration/StorePortRaceTests.cs @@ -30,6 +30,7 @@ public async Task TwoProcessesStartingStoresAtOnce_EachKeepTheirOwnStore() var results = await Task.WhenAll(children); foreach (var r in results) output.WriteLine($"exit={r.Exit} {r.Line}"); + results.Should().HaveCount(2, "the claims below hold vacuously over one result (rev-reach 9de418b9d)"); results.Should().OnlyContain(r => r.Exit == 0 && r.Line.EndsWith("RESULT=SURVIVED"), "each process keeps its own store"); results.Select(r => Field(r.Line, "PORT")).Should().OnlyHaveUniqueItems("two processes never share a port"); results.Select(r => Field(r.Line, "CONTAINER")).Should().OnlyHaveUniqueItems("nor a container"); diff --git a/src/MicroPlumberd.Tests/Unit/PortLockTests.cs b/src/MicroPlumberd.Tests/Unit/PortLockTests.cs new file mode 100644 index 0000000..6928c74 --- /dev/null +++ b/src/MicroPlumberd.Tests/Unit/PortLockTests.cs @@ -0,0 +1,56 @@ +using FluentAssertions; +using MicroPlumberd.Testing; + +namespace MicroPlumberd.Tests.Unit; + +/// +/// rev-reach 9de418b9d m1: the port-lock directory is shared by every USER of a machine. Each row uses its OWN directory +/// (the default is shared with every parallel class's stores), and its own port range. +/// +public class PortLockTests : IDisposable +{ + private readonly string _dir = Path.Combine(Directory.CreateTempSubdirectory("mp-portlock-").FullName, "ports"); + + public void Dispose() + { + var root = Path.GetDirectoryName(_dir)!; + if (!OperatingSystem.IsWindows() && Directory.Exists(_dir)) File.SetUnixFileMode(_dir, (UnixFileMode)0b111_111_111); + Directory.Delete(root, recursive: true); + } + + [Fact] + public void TheDirectoryAndItsLockFiles_AreSharedWithEveryUser_WhateverTheUmask() + { + if (OperatingSystem.IsWindows()) return; + var (port, held) = PortLock.Acquire(new PortSearcher(41_000), _dir); + using (held) + { + File.GetUnixFileMode(_dir).Should().Be((UnixFileMode)0b1_111_111_111, "1777: any user may add a lock, none may remove another's"); + File.GetUnixFileMode(Path.Combine(_dir, $"{port}.lock")).Should().Be((UnixFileMode)0b110_110_110, "0666: any user may lock it later"); + } + } + + [Fact] + public void ALockFileThisUserMayNotOpen_IsAPortToSkip_NotAThrow() + { + if (OperatingSystem.IsWindows()) return; + Directory.CreateDirectory(_dir); + File.WriteAllText(Path.Combine(_dir, "42001.lock"), ""); + File.SetUnixFileMode(Path.Combine(_dir, "42001.lock"), UnixFileMode.UserRead); // as another user's 0644 file is to this one + + var (port, held) = PortLock.Acquire(new PortSearcher(42_000), _dir); + using (held) port.Should().BeGreaterThan(42_001, "the port whose lock file this user may not open is skipped"); + } + + [Fact] + public void ADirectoryInWhichNothingCanBeLocked_IsRefusedNamed_NeverAnUnhandledThrow() + { + if (OperatingSystem.IsWindows()) return; + Directory.CreateDirectory(_dir); + File.SetUnixFileMode(_dir, (UnixFileMode)0b101_101_101); // 0555 — as another user's 0755 directory is to this one + + var acquire = () => PortLock.Acquire(new PortSearcher(43_000), _dir); + + acquire.Should().Throw().WithMessage($"*'{_dir}'*chmod 1777*"); + } +} From cab19595c6e1378f22c7efef9c620ab8b2b8d14c Mon Sep 17 00:00:00 2001 From: Rafal Maciag Date: Sun, 18 Oct 2026 17:56:33 +0000 Subject: [PATCH 2/2] =?UTF-8?q?docs(testing):=20PortLock=20says=20why=20lo?= =?UTF-8?q?ck=20files=20are=20never=20tidied=20while=20tests=20run=20(an?= =?UTF-8?q?=20unlink=20while=20held=20swaps=20the=20inode=20and=20the=20ra?= =?UTF-8?q?ce=20returns),=20and=20the=20refusal=20names=20another=20user's?= =?UTF-8?q?=20stale=20lock=20files=20as=20a=20cause=20(rev-reach=20fbe88ab?= =?UTF-8?q?06)=20=E2=80=94=20text=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- src/MicroPlumberd.Testing/PortLock.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/MicroPlumberd.Testing/PortLock.cs b/src/MicroPlumberd.Testing/PortLock.cs index d0a20f5..64647ba 100644 --- a/src/MicroPlumberd.Testing/PortLock.cs +++ b/src/MicroPlumberd.Testing/PortLock.cs @@ -12,6 +12,9 @@ namespace MicroPlumberd.Testing; /// and never an endless search. /// /// +// Never tidy lock files while tests run: this class never unlinks one (Dispose only closes it, releasing the flock). A lock +// file removed while held — only its owner, the directory's owner or root can, under the sticky bit — lets the next opener +// create a NEW inode and hold it too, and the race is back. internal sealed class PortLock : IDisposable { /// One directory for every process on the machine (not the per-user/per-process TMPDIR). @@ -50,7 +53,8 @@ internal static (int Port, PortLock Lock) Acquire(PortSearcher searcher, string? case Denied when ++denied >= DeniedLimit: throw new InvalidOperationException( $"No store port could be locked in '{directory}': this user may not create or open lock files there " + - $"({DeniedLimit} ports in a row). Make it shared — chmod 1777 '{directory}' — or remove it so it is recreated."); + $"({DeniedLimit} ports in a row). Make it shared — chmod 1777 '{directory}' — or remove it so it is recreated; " + + "stale lock files of another user (made before these modes) are the other cause: remove them."); case Denied: continue; default: denied = 0; continue; // held by another process: the next port }