Skip to content

Commit e4b3d0d

Browse files
committed
Refactor GameProcess to use Volatile and Interlocked for thread-safe process termination handling.
1 parent 3c74c1c commit e4b3d0d

1 file changed

Lines changed: 13 additions & 9 deletions

File tree

src/PG.StarWarsGame.Infrastructure/Clients/Processes/GameProcess.cs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace PG.StarWarsGame.Infrastructure.Clients.Processes;
99

1010
internal sealed class GameProcess : DisposableObject, IGameProcess
1111
{
12-
private volatile bool _closed;
12+
private int _closedFlag;
1313

1414
private EventHandler? _closingHandler;
1515

@@ -18,7 +18,7 @@ public event EventHandler? Closed
1818
add
1919
{
2020
// Execute event right away if the process was already closed.
21-
if (_closed)
21+
if (Volatile.Read(ref _closedFlag) != 0)
2222
value?.Invoke(this, EventArgs.Empty);
2323
else
2424
_closingHandler += value;
@@ -30,8 +30,8 @@ public event EventHandler? Closed
3030

3131
// To avoid complexity this property does not query the underlying process but just "trusts"
3232
// on the OnClosed function to set the value when the process was terminated.
33-
// This means this property has theoretically a race condition with the game's process.
34-
public GameProcessState State => _closed ? GameProcessState.Closed : GameProcessState.Running;
33+
// This means this property has theoretically a race condition with the game's process.
34+
public GameProcessState State => Volatile.Read(ref _closedFlag) != 0 ? GameProcessState.Closed : GameProcessState.Running;
3535

3636
internal Process Process { get; }
3737

@@ -51,10 +51,16 @@ public void Exit()
5151
try
5252
{
5353
Process.Kill();
54+
Process.WaitForExit();
5455
}
5556
catch (Exception e) when (e is InvalidOperationException or Win32Exception)
5657
{
5758
}
59+
60+
// Process.Exited is dispatched asynchronously on a ThreadPool work item, so
61+
// WaitForExit can return before OnClosed has been invoked. Drive it synchronously
62+
// here; OnClosed is idempotent via the Interlocked guard.
63+
OnClosed(this, EventArgs.Empty);
5864
}
5965

6066
public Task WaitForExitAsync(CancellationToken cancellationToken = default)
@@ -74,15 +80,13 @@ private void RegisterExitEvent(Process process)
7480
Process.EnableRaisingEvents = true;
7581
Process.Exited += OnClosed;
7682
if (process.HasExited)
77-
{
78-
_closed = true;
79-
Process.Exited -= OnClosed;
80-
}
83+
OnClosed(this, EventArgs.Empty);
8184
}
8285

8386
private void OnClosed(object? sender, EventArgs e)
8487
{
85-
_closed = true;
88+
if (Interlocked.Exchange(ref _closedFlag, 1) != 0)
89+
return;
8690
Process.Exited -= OnClosed;
8791
_closingHandler?.Invoke(this, EventArgs.Empty);
8892
}

0 commit comments

Comments
 (0)