Skip to content

Commit 08b1cc7

Browse files
sebgodclaude
andcommitted
fix(instance-gate): exclusive between processes on Linux and macOS
The pipe was the lock only on Windows. .NET's Unix named-pipe server is a Unix-domain socket that unlinks whatever socket is at its path and binds its own, and it enforces maxNumberOfServerInstances only within one process (NamedPipeServerStream.Unix.cs, SharedServer). So a second process claimed the gate instead of being refused: every launch held it, a hand-off reached the newest window, and the older gate's dispose unlinked the newer one's socket. Off Windows the claim is now an exclusive advisory lock (flock, which .NET takes for FileShare.None) on <channel>.claim in the temporary folder, held for the gate's life and released after the server is disposed. The kernel drops it when the process dies however it dies, so a crash leaves no stale claim, which FirstPipeInstance alone would: its bind fails on a dead holder's socket file for good. The existing exclusivity test held both claims in one process, where .NET refuses the second server by itself, so it passed on Linux all along. Two new tests hold the gate from a second process (the test assembly run again, claiming in a module initializer): another process's claim refuses this one and still takes its hand-off, and a killed holder leaves the claim free. The first failed on Linux (WSL, Ubuntu 24.04, .NET 10.0.112) before the lock and passes three runs in a row after it; Windows passes both ways. Found by TianWen's review #1414. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YTPcimK4UctQtAyuNCZy3z
1 parent b7edbf9 commit 08b1cc7

5 files changed

Lines changed: 169 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@ Release notes live here rather than beside the version number: the number is one
66

77
## 1.1
88

9+
- **`InstanceGate` is exclusive on Linux and macOS too.** It never was: .NET's named-pipe server
10+
there is a Unix-domain socket that unlinks whatever socket is at its path and binds its own,
11+
enforcing the single instance only within one process. So a second launch claimed the gate
12+
instead of being refused, a hand-off reached the newest window rather than the first, and the
13+
older gate's dispose then unlinked the newer one's socket. Off Windows the claim is now an
14+
advisory lock (`flock`) on a `<channel>.claim` file in the temporary folder, held for the gate's
15+
life and released by the kernel when the process dies, so a crashed holder leaves no stale claim.
16+
Pinned by two tests that hold the gate from a second process, which fail on Linux without the
17+
lock. No API change.
918
- **`InstanceGate.TryHandOff` answers "nobody is there" at once instead of at the timeout.**
1019
`NamedPipeClientStream.Connect` does not distinguish an absent holder from a busy one: it polls
1120
for the name to appear and gives up only when the timeout expires. Waiting is right for a busy

‎README.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,11 @@ and `c:\data\` do not open two windows onto the same folder.
5858

5959
## Three things it gets right that are easy to get wrong
6060

61-
**The pipe is the lock.** A named pipe with a single server instance can only be created once, so
62-
claiming it *is* the primacy test — there is no separate mutex, one lifetime to get right, and no
63-
abandoned-mutex case.
61+
**On Windows the pipe is the lock.** A named pipe with a single server instance can only be created
62+
once, so claiming it *is* the primacy test — there is no separate mutex, one lifetime to get right,
63+
and no abandoned-mutex case. On Linux and macOS .NET's pipe is a Unix-domain socket whose server
64+
replaces whatever socket is at its path, so there the claim is an advisory lock (`flock`) on a file
65+
beside it, which the kernel releases when its process dies, a crash included.
6466

6567
**The accept loop is not on the thread pool.** The pipe is deliberately not `Asynchronous`, and the
6668
accept runs on a dedicated thread. An awaited accept resumes on a pool worker, and an app of this
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
using System.Diagnostics;
2+
using System.Runtime.CompilerServices;
3+
4+
namespace SharpAstro.AppShell.Tests;
5+
6+
/// <summary>
7+
/// A second PROCESS holding a gate. A claim's exclusivity is a property of the OS between processes,
8+
/// and inside one process .NET refuses a second server of one name by itself (its Unix server keeps a
9+
/// per-process table), so a test of two gates in one process passes whether or not two processes
10+
/// would be kept apart.
11+
///
12+
/// <para>The holder is this test assembly run again, <c>dotnet SharpAstro.AppShell.Tests.dll</c>, with
13+
/// <see cref="ChannelVariable"/> naming the channel: the module initializer below claims it before the
14+
/// test runner starts, prints <c>claimed</c> or <c>taken</c>, holds the claim until its standard input
15+
/// closes, and exits without running a test.</para>
16+
/// </summary>
17+
internal sealed class GateHolderProcess : IDisposable
18+
{
19+
private const string ChannelVariable = "APPSHELL_TESTS_HOLD_GATE";
20+
21+
private readonly Process _process;
22+
23+
private GateHolderProcess(Process process) => _process = process;
24+
25+
/// <summary>What the holder answered: <c>claimed</c> or <c>taken</c>.</summary>
26+
public string Answer { get; private init; } = "";
27+
28+
public static async Task<GateHolderProcess> StartAsync(string channel, CancellationToken cancellationToken)
29+
{
30+
var info = new ProcessStartInfo(DotnetHost(), $"\"{typeof(GateHolderProcess).Assembly.Location}\"")
31+
{
32+
RedirectStandardInput = true,
33+
RedirectStandardOutput = true,
34+
UseShellExecute = false,
35+
};
36+
info.Environment[ChannelVariable] = channel;
37+
var process = Process.Start(info) ?? throw new InvalidOperationException("The holder process did not start.");
38+
var answer = await process.StandardOutput.ReadLineAsync(cancellationToken) ?? "";
39+
return new GateHolderProcess(process) { Answer = answer };
40+
}
41+
42+
/// <summary>Ends the holder as a crash does: nothing it would do on its way out runs.</summary>
43+
public async Task KillAsync(CancellationToken cancellationToken)
44+
{
45+
_process.Kill(entireProcessTree: true);
46+
await _process.WaitForExitAsync(cancellationToken);
47+
}
48+
49+
public void Dispose()
50+
{
51+
if (!_process.HasExited)
52+
{
53+
_process.StandardInput.Close();
54+
if (!_process.WaitForExit(10_000))
55+
{
56+
_process.Kill(entireProcessTree: true);
57+
}
58+
}
59+
_process.Dispose();
60+
}
61+
62+
// The SDK's own host when a test runs under it, else the one on PATH
63+
private static string DotnetHost()
64+
=> Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") is { Length: > 0 } host ? host : "dotnet";
65+
66+
[ModuleInitializer]
67+
internal static void HoldWhenAsked()
68+
{
69+
if (Environment.GetEnvironmentVariable(ChannelVariable) is not { Length: > 0 } channel)
70+
{
71+
return;
72+
}
73+
74+
using (var gate = InstanceGate.TryClaim(channel))
75+
{
76+
Console.Out.WriteLine(gate is null ? "taken" : "claimed");
77+
Console.Out.Flush();
78+
Console.In.ReadToEnd();
79+
}
80+
Environment.Exit(0);
81+
}
82+
}

‎src/SharpAstro.AppShell.Tests/InstanceGateTests.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,39 @@ public void The_first_claim_wins_and_the_second_is_refused()
5656
InstanceGate.TryClaim(channel).ShouldBeNull();
5757
}
5858

59+
[Fact(Timeout = 60_000)]
60+
public async Task A_claim_another_process_holds_refuses_this_one()
61+
{
62+
// The test above holds both claims in one process, where .NET refuses the second server by
63+
// itself. Between processes on Linux and macOS, .NET's pipe server unlinked the holder's
64+
// socket and bound its own, so EVERY launch claimed the gate and a hand-off reached the
65+
// newest window rather than the first.
66+
var ct = TestContext.Current.CancellationToken;
67+
var channel = InstanceGate.ChannelFor(Scope, UniqueIdentity("cross-process"));
68+
69+
using var holder = await GateHolderProcess.StartAsync(channel, ct);
70+
holder.Answer.ShouldBe("claimed");
71+
72+
InstanceGate.TryClaim(channel).ShouldBeNull();
73+
InstanceGate.TryHandOff(channel, "to the holder", TimeSpan.FromSeconds(5)).ShouldBeTrue();
74+
}
75+
76+
[Fact(Timeout = 60_000)]
77+
public async Task A_holder_that_died_leaves_the_claim_free()
78+
{
79+
// A crash runs no dispose, so whatever marks the claim must die with the process, or a
80+
// crashed window would refuse every later launch the gate.
81+
var ct = TestContext.Current.CancellationToken;
82+
var channel = InstanceGate.ChannelFor(Scope, UniqueIdentity("crashed"));
83+
84+
using var holder = await GateHolderProcess.StartAsync(channel, ct);
85+
holder.Answer.ShouldBe("claimed");
86+
await holder.KillAsync(ct);
87+
88+
using var next = InstanceGate.TryClaim(channel);
89+
next.ShouldNotBeNull();
90+
}
91+
5992
[Fact]
6093
public void Releasing_a_claim_lets_the_next_process_take_it()
6194
{

‎src/SharpAstro.AppShell/InstanceGate.cs‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,22 @@ namespace SharpAstro.AppShell;
3131
/// open folder", where opening a file in a new folder gets a new window but opening one in a folder
3232
/// already on screen activates that window. Neither policy is baked in here.</para>
3333
///
34-
/// <para><b>The pipe is the lock.</b> A named pipe with a single server instance can only be
34+
/// <para><b>On Windows the pipe is the lock.</b> A named pipe with a single server instance can only be
3535
/// created once, so <see cref="TryClaim"/> succeeding IS the claim, and the same object then
3636
/// carries the hand-off traffic. One primitive means one lifetime to get right and no
3737
/// abandoned-mutex case to reason about.</para>
3838
///
39+
/// <para><b>On Linux and macOS it cannot be, so a lock file is.</b> .NET's pipe there is a Unix-domain
40+
/// socket, and its server unlinks whatever socket is at the path and binds its own, enforcing the
41+
/// single instance only within one process (<c>NamedPipeServerStream.Unix.cs</c>, <c>SharedServer</c>).
42+
/// So a second process took the name instead of being refused: every launch claimed the gate, a
43+
/// hand-off reached the NEWEST window, and the older gate's dispose then unlinked the newer one's
44+
/// socket. The claim is an exclusive advisory lock (<c>flock</c>, which .NET takes for
45+
/// <see cref="FileShare.None"/>) on a file beside the socket, held for the gate's life; the kernel
46+
/// releases it when its process dies, however it dies, so a crash leaves no stale claim behind, which
47+
/// <see cref="PipeOptions.FirstPipeInstance"/> alone would (its bind fails on a dead holder's socket
48+
/// file for good).</para>
49+
///
3950
/// <para><b>The accept loop gets its own thread, and the pipe is deliberately NOT
4051
/// <see cref="PipeOptions.Asynchronous"/>.</b> An awaited accept resumes on a thread-pool worker,
4152
/// and a desktop app of this kind saturates its own pool with decode and tessellation work -- so
@@ -65,6 +76,9 @@ public sealed class InstanceGate : IDisposable
6576

6677
// Not readonly: a wedged instance is replaced, see AcceptLoop. Written only by the accept thread.
6778
private volatile NamedPipeServerStream _server;
79+
80+
// The claim off Windows, held until after the server is disposed (see the class remarks); null on Windows.
81+
private readonly FileStream? _claim;
6882
private readonly ConcurrentQueue<HandoffRequest> _incoming = new();
6983
private readonly ILogger? _log;
7084
private Thread? _thread;
@@ -74,9 +88,10 @@ public sealed class InstanceGate : IDisposable
7488
// connection (see Dispose), and this is what tells it that connection was ours.
7589
private volatile bool _stopping;
7690

77-
private InstanceGate(NamedPipeServerStream server, string channel, ILogger? log)
91+
private InstanceGate(NamedPipeServerStream server, FileStream? claim, string channel, ILogger? log)
7892
{
7993
_server = server;
94+
_claim = claim;
8095
Channel = channel;
8196
_log = log;
8297
}
@@ -140,11 +155,19 @@ public static string NormalizePathIdentity(string path)
140155
{
141156
ArgumentException.ThrowIfNullOrWhiteSpace(channel);
142157

158+
FileStream? claim = null;
143159
NamedPipeServerStream server;
144160
try
145161
{
146-
// maxNumberOfServerInstances 1 is what makes creation exclusive, and therefore what
147-
// makes this the primacy test rather than merely a transport.
162+
if (!OperatingSystem.IsWindows())
163+
{
164+
// The claim itself (see the class remarks); an IOException is another process holding it
165+
claim = new FileStream(ClaimPath(channel), FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
166+
}
167+
168+
// maxNumberOfServerInstances 1 is what makes creation exclusive on Windows, and therefore
169+
// what makes this the primacy test there rather than merely a transport. Elsewhere the
170+
// claim above is, and this server only replaces whatever socket a dead holder left.
148171
server = new NamedPipeServerStream(
149172
channel,
150173
PipeDirection.InOut,
@@ -155,17 +178,19 @@ public static string NormalizePathIdentity(string path)
155178
catch (IOException)
156179
{
157180
// The name is taken: somebody else is the instance for this identity.
181+
claim?.Dispose();
158182
return null;
159183
}
160184
catch (Exception ex)
161185
{
162186
// Anything else (an unsupported platform, a permissions problem) means we cannot gate.
163187
// Running without one is correct: the app opens its own window.
188+
claim?.Dispose();
164189
log?.LogDebug(ex, "Could not claim instance channel {Channel}; continuing ungated", channel);
165190
return null;
166191
}
167192

168-
var gate = new InstanceGate(server, channel, log);
193+
var gate = new InstanceGate(server, claim, channel, log);
169194
gate._thread = new Thread(gate.AcceptLoop)
170195
{
171196
IsBackground = true,
@@ -175,6 +200,10 @@ public static string NormalizePathIdentity(string path)
175200
return gate;
176201
}
177202

203+
// Beside .NET's own socket for the channel, in the per-user temporary folder on macOS; on Linux the
204+
// channel's hash of the user's name keeps two users' claims apart in a shared /tmp
205+
private static string ClaimPath(string channel) => Path.Combine(Path.GetTempPath(), $"{channel}.claim");
206+
178207
/// <summary>
179208
/// Hand <paramref name="payload"/> to the instance holding <paramref name="channel"/>, and
180209
/// grant it the right to raise its window. Returns false if nobody answered in time, in which
@@ -400,7 +429,8 @@ private void AcceptLoop()
400429

401430
/// <summary>
402431
/// Rebuilds the server instance after it has stopped accepting. Returns false when the name
403-
/// cannot be retaken, which means somebody else now holds it and this gate is finished.
432+
/// cannot be retaken, which means somebody else now holds it and this gate is finished. Off
433+
/// Windows the claim is held throughout, so nobody can take the name in between.
404434
/// </summary>
405435
private bool TryReplaceWedgedServer()
406436
{
@@ -478,6 +508,10 @@ public void Dispose()
478508

479509
_thread?.Join(TimeSpan.FromSeconds(2));
480510
_server.Dispose();
511+
512+
// Only now: disposing the server unlinks the socket path, which must not happen to a socket a
513+
// newer holder has already bound there
514+
_claim?.Dispose();
481515
}
482516

483517
private static string Sanitize(string value)

0 commit comments

Comments
 (0)