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..64647ba 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,94 @@ 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. +/// /// +// 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). - 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; " + + "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 + } } } - 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*"); + } +}