From 7bd41280689e83e2081e33334c880f3fa29841f1 Mon Sep 17 00:00:00 2001 From: AriusII Date: Mon, 21 Sep 2026 19:01:10 +0200 Subject: [PATCH] Fix process try failure semantics --- .../Dispatching/ICheatEngineDispatcher.cs | 8 +- .../Processes/IProcessClient.cs | 11 + .../Domains/ProcessClient.cs | 126 +++++++--- .../SdkMainThreadDispatcherBehaviorTests.cs | 10 +- .../Domains/ProcessClientTests.cs | 231 +++++++++++++++--- 5 files changed, 314 insertions(+), 72 deletions(-) diff --git a/libs/CheatEngine.Client.Abstractions/Dispatching/ICheatEngineDispatcher.cs b/libs/CheatEngine.Client.Abstractions/Dispatching/ICheatEngineDispatcher.cs index 080f467..825172c 100644 --- a/libs/CheatEngine.Client.Abstractions/Dispatching/ICheatEngineDispatcher.cs +++ b/libs/CheatEngine.Client.Abstractions/Dispatching/ICheatEngineDispatcher.cs @@ -17,12 +17,18 @@ public bool IsMainThread /// /// is observed before dispatch admission only. It never attempts to /// interrupt a callback or Lua primitive that has already begun on Cheat Engine's main thread. + /// A result represents cancellation or a dispatcher admission/infrastructure failure. + /// Exceptions thrown by are rethrown unchanged. /// public bool TryInvoke(Action callback, out CheatEngineFailure failure, CancellationToken cancellationToken = default); /// Runs a callback on the captured main thread and returns its managed result. - /// Cancellation is observed before dispatch admission and never while the callback is running. + /// + /// Cancellation is observed before dispatch admission and never while the callback is running. A + /// result represents cancellation or a dispatcher admission/infrastructure failure; + /// exceptions thrown by are rethrown unchanged. + /// public bool TryInvoke(Func callback, [MaybeNullWhen(false)] out T result, out CheatEngineFailure failure, CancellationToken cancellationToken = default); diff --git a/libs/CheatEngine.Client.Abstractions/Processes/IProcessClient.cs b/libs/CheatEngine.Client.Abstractions/Processes/IProcessClient.cs index f90e41c..ee46fbd 100644 --- a/libs/CheatEngine.Client.Abstractions/Processes/IProcessClient.cs +++ b/libs/CheatEngine.Client.Abstractions/Processes/IProcessClient.cs @@ -15,6 +15,12 @@ public ProcessEnumerationResult GetProcesses(ProcessEnumerationRequest request, CancellationToken cancellationToken = default); /// Tries to get a copied snapshot of the currently selected target process. + /// + /// Returns when Cheat Engine has no selected target or + /// its selected target is no longer available in local process metadata. An inconsistent local metadata result + /// returns . Invalid arguments, lifecycle failures, and + /// unexpected implementation exceptions are not converted into a Try result. + /// public bool TryGetCurrent(out ProcessSnapshot snapshot, out CheatEngineFailure failure, CancellationToken cancellationToken = default); @@ -25,6 +31,11 @@ public bool TryGetCurrent(out ProcessSnapshot snapshot, out CheatEngineFailure f /// Re-reads Cheat Engine's selected target and advances the selection epoch when its PID or observed /// architecture changed. /// + /// + /// Returns and invalidates an observed selection when + /// the selected target is absent or no longer has local process metadata. This is an observation, not an + /// atomic process-lifetime guarantee. + /// public bool TryRefresh(out ProcessSnapshot snapshot, out CheatEngineFailure failure, CancellationToken cancellationToken = default); diff --git a/libs/CheatEngine.Client.Core/Domains/ProcessClient.cs b/libs/CheatEngine.Client.Core/Domains/ProcessClient.cs index 639152d..1056b48 100644 --- a/libs/CheatEngine.Client.Core/Domains/ProcessClient.cs +++ b/libs/CheatEngine.Client.Core/Domains/ProcessClient.cs @@ -157,25 +157,35 @@ public bool TryAttach( throw new ArgumentOutOfRangeException(nameof(processId)); } - ProcessSnapshot captured = default; + CurrentProcessCapture captured = default; if (!_dispatcher.TryInvoke( - () => - { - _host.OpenProcess(processId.Value); - captured = CaptureCurrent("Processes.Attach"); - if (captured.Id != processId) - { - throw new InvalidOperationException("Cheat Engine did not select the requested process."); - } - }, - out failure, - cancellationToken)) + () => + { + _host.OpenProcess(processId.Value); + captured = CaptureCurrent("Processes.Attach"); + }, + out failure, + cancellationToken)) { snapshot = default; return false; } - snapshot = captured; + if (!TryGetCapturedSnapshot(captured, "Processes.Attach", out snapshot, out failure)) + { + return false; + } + + if (snapshot.Id != processId) + { + snapshot = default; + failure = new CheatEngineFailure( + CheatEngineFailureKind.OperationRejected, + "Processes.Attach", + "Cheat Engine did not select the requested process."); + return false; + } + return true; } @@ -336,53 +346,77 @@ private bool TryReadCurrent( out CheatEngineFailure failure, CancellationToken cancellationToken) { - ProcessSnapshot captured = default; + CurrentProcessCapture captured = default; if (!_dispatcher.TryInvoke(() => captured = CaptureCurrent(operation), out failure, cancellationToken)) { snapshot = default; - if (failure.Kind == CheatEngineFailureKind.OperationRejected) - { - failure = new CheatEngineFailure( - CheatEngineFailureKind.TargetNotAttached, - operation, - failure.Message, - failure.Exception); - } - return false; } - snapshot = captured; - return true; + return TryGetCapturedSnapshot(captured, operation, out snapshot, out failure); } - private ProcessSnapshot CaptureCurrent(string operation) + private CurrentProcessCapture CaptureCurrent(string operation) { long processId = _host.GetOpenedProcessId(); if (processId is <= 0 or > int.MaxValue) { ClearObservedSelection(operation); - throw new InvalidOperationException("Cheat Engine has no selected local target process."); + return new CurrentProcessCapture(CurrentProcessCaptureFailure.NoTargetSelected); } TargetProcessId id = new(checked((int) processId)); if (!_host.TryGetLocalProcess(id.Value, out LocalProcessInfo process)) { ClearObservedSelection(operation); - throw new InvalidOperationException("The selected process no longer exists locally."); + return new CurrentProcessCapture(CurrentProcessCaptureFailure.LocalProcessUnavailable); } if (process.Id != id.Value) { - throw new EngineMarshallingException( - "Processes.GetCurrent", - EngineMarshallingDirection.Result, - "metadata for the selected process identifier", - $"metadata for process {process.Id}"); + return new CurrentProcessCapture(CurrentProcessCaptureFailure.InvalidLocalMetadata, process.Id); } CheatEngineArchitecture architecture = TryGetTargetArchitecture(); - return ObserveSelection(id, process, architecture, operation); + return new CurrentProcessCapture(ObserveSelection(id, process, architecture, operation)); + } + + private static bool TryGetCapturedSnapshot( + CurrentProcessCapture captured, + string operation, + out ProcessSnapshot snapshot, + out CheatEngineFailure failure) + { + if (captured.Failure == CurrentProcessCaptureFailure.None) + { + snapshot = captured.Snapshot; + failure = default; + return true; + } + + snapshot = default; + failure = captured.Failure switch + { + CurrentProcessCaptureFailure.NoTargetSelected => new CheatEngineFailure( + CheatEngineFailureKind.TargetNotAttached, + operation, + "Cheat Engine has no selected local target process."), + CurrentProcessCaptureFailure.LocalProcessUnavailable => new CheatEngineFailure( + CheatEngineFailureKind.TargetNotAttached, + operation, + "The selected target process is no longer available in local process metadata."), + CurrentProcessCaptureFailure.InvalidLocalMetadata => new CheatEngineFailure( + CheatEngineFailureKind.InvalidHostResult, + operation, + "The selected target's local process metadata did not match its identifier.", + new EngineMarshallingException( + operation, + EngineMarshallingDirection.Result, + "metadata for the selected process identifier", + $"metadata for process {captured.ObservedProcessId}")), + _ => throw new InvalidOperationException("The current process capture produced an unknown failure.") + }; + return false; } private CheatEngineArchitecture TryGetTargetArchitecture() @@ -518,4 +552,28 @@ private static CheatEngineFailure Cancelled(string operation) } private readonly record struct ProcessSelection(TargetProcessId Id, CheatEngineArchitecture Architecture); + + private readonly record struct CurrentProcessCapture( + ProcessSnapshot Snapshot, + CurrentProcessCaptureFailure Failure, + int ObservedProcessId) + { + internal CurrentProcessCapture(ProcessSnapshot snapshot) + : this(snapshot, CurrentProcessCaptureFailure.None, 0) + { + } + + internal CurrentProcessCapture(CurrentProcessCaptureFailure failure, int observedProcessId = 0) + : this(default, failure, observedProcessId) + { + } + } + + private enum CurrentProcessCaptureFailure + { + None, + NoTargetSelected, + LocalProcessUnavailable, + InvalidLocalMetadata + } } diff --git a/tests/CheatEngine.Client.Core.Tests/Dispatching/SdkMainThreadDispatcherBehaviorTests.cs b/tests/CheatEngine.Client.Core.Tests/Dispatching/SdkMainThreadDispatcherBehaviorTests.cs index f7b9481..afc8420 100644 --- a/tests/CheatEngine.Client.Core.Tests/Dispatching/SdkMainThreadDispatcherBehaviorTests.cs +++ b/tests/CheatEngine.Client.Core.Tests/Dispatching/SdkMainThreadDispatcherBehaviorTests.cs @@ -57,16 +57,18 @@ public void CallbackExceptionsAreRethrownWithoutBeingClassifiedAsHostFailures() using ControlledCoreLifetimeContext context = new(); using CoreLifetime lifetime = new(context); SdkMainThreadDispatcher dispatcher = new(lifetime, new RecordingMainThreadInvoker()); + InvalidOperationException expectedActionException = new("action callback"); + InvalidOperationException expectedFunctionException = new("function callback"); InvalidOperationException actionException = Assert.Throws(() => - dispatcher.TryInvoke(static () => throw new InvalidOperationException("action callback"), + dispatcher.TryInvoke(() => throw expectedActionException, out CheatEngineFailure _, TestContext.Current.CancellationToken)); InvalidOperationException functionException = Assert.Throws(() => - dispatcher.TryInvoke(static () => throw new InvalidOperationException("function callback"), out _, + dispatcher.TryInvoke(() => throw expectedFunctionException, out _, out CheatEngineFailure _, TestContext.Current.CancellationToken)); - Assert.Equal("action callback", actionException.Message); - Assert.Equal("function callback", functionException.Message); + Assert.Same(expectedActionException, actionException); + Assert.Same(expectedFunctionException, functionException); } [Fact] diff --git a/tests/CheatEngine.Client.Core.Tests/Domains/ProcessClientTests.cs b/tests/CheatEngine.Client.Core.Tests/Domains/ProcessClientTests.cs index 37a693b..b580f4b 100644 --- a/tests/CheatEngine.Client.Core.Tests/Domains/ProcessClientTests.cs +++ b/tests/CheatEngine.Client.Core.Tests/Domains/ProcessClientTests.cs @@ -1,8 +1,11 @@ using CheatEngine.Client.Core.Domains; +using CheatEngine.Client.Core.Dispatching; using CheatEngine.Client.Core.Infrastructure; +using CheatEngine.Client.Core.Tests.TestSupport; using CheatEngine.Client.Dispatching; using CheatEngine.Client.Processes; using CheatEngine.Client.Results; +using CheatEngine.SDK.Engine.Errors; using CheatEngine.SDK.Engine.Inspection; using CheatEngine.SDK.Engine.Runtime; @@ -147,8 +150,13 @@ public void RefreshChangesArchitectureForTheSamePidAndAdvancesSelectionEpoch() public void TryGetCurrentReportsTargetNotAttachedAndInvalidatesTheKnownSelectionWhenCeDetaches() { FakeProcessHost host = FakeProcessHost.CreateSelected(42, CheatEngineArchitecture.X64); + using ControlledCoreLifetimeContext activationContext = new(); + using CoreLifetime activationLifetime = new(activationContext); using TargetSelectionLifetime selectionLifetime = CreateSelectionLifetime(); - ProcessClient client = new(new InlineDispatcher(), host, selectionLifetime); + ProcessClient client = new( + new SdkMainThreadDispatcher(activationLifetime, new InlineMainThreadInvoker()), + host, + selectionLifetime); ProcessSnapshot initial = client.GetCurrent(TestContext.Current.CancellationToken); host.OpenedProcessId = 0; @@ -160,6 +168,7 @@ public void TryGetCurrentReportsTargetNotAttachedAndInvalidatesTheKnownSelection Assert.False(succeeded); Assert.Equal(default, snapshot); Assert.Equal(CheatEngineFailureKind.TargetNotAttached, failure.Kind); + Assert.Equal("Processes.GetCurrent", failure.Operation); Assert.Equal(1, selectionLifetime.Epoch); Assert.Throws(() => selectionLifetime.ThrowIfExpired(initial.SelectionEpoch, "Test.TargetLease")); @@ -169,14 +178,137 @@ public void TryGetCurrentReportsTargetNotAttachedAndInvalidatesTheKnownSelection Assert.Equal(CheatEngineFailureKind.TargetNotAttached, exception.Failure.Kind); } + [Fact] + public void TryRefreshReportsTargetNotAttachedOnceWhenTheSelectedProcessDisappears() + { + FakeProcessHost host = FakeProcessHost.CreateSelected(42, CheatEngineArchitecture.X64); + using ControlledCoreLifetimeContext activationContext = new(); + using CoreLifetime activationLifetime = new(activationContext); + using TargetSelectionLifetime selectionLifetime = CreateSelectionLifetime(); + ProcessClient client = new( + new SdkMainThreadDispatcher(activationLifetime, new InlineMainThreadInvoker()), + host, + selectionLifetime); + ProcessSnapshot initial = client.GetCurrent(TestContext.Current.CancellationToken); + RecordingDisposable lease = new(); + selectionLifetime.Track(lease, initial.SelectionEpoch); + host.LocalProcesses.Remove(initial.Id.Value); + + bool firstSucceeded = client.TryRefresh( + out ProcessSnapshot firstSnapshot, + out CheatEngineFailure firstFailure, + TestContext.Current.CancellationToken); + bool secondSucceeded = client.TryRefresh( + out ProcessSnapshot secondSnapshot, + out CheatEngineFailure secondFailure, + TestContext.Current.CancellationToken); + + Assert.False(firstSucceeded); + Assert.Equal(default, firstSnapshot); + Assert.Equal(CheatEngineFailureKind.TargetNotAttached, firstFailure.Kind); + Assert.Equal("Processes.Refresh", firstFailure.Operation); + Assert.False(secondSucceeded); + Assert.Equal(default, secondSnapshot); + Assert.Equal(CheatEngineFailureKind.TargetNotAttached, secondFailure.Kind); + Assert.Equal("Processes.Refresh", secondFailure.Operation); + Assert.Equal(1, selectionLifetime.Epoch); + Assert.Equal(1, lease.DisposeCount); + } + + [Fact] + public void TryGetCurrentReportsInvalidHostResultForInconsistentLocalMetadata() + { + FakeProcessHost host = FakeProcessHost.CreateSelected(42, CheatEngineArchitecture.X64); + host.LocalProcesses[42] = new LocalProcessInfo(41, "fixture", "C:\\fixtures\\fixture.exe"); + using ControlledCoreLifetimeContext activationContext = new(); + using CoreLifetime activationLifetime = new(activationContext); + using TargetSelectionLifetime selectionLifetime = CreateSelectionLifetime(); + ProcessClient client = new( + new SdkMainThreadDispatcher(activationLifetime, new InlineMainThreadInvoker()), + host, + selectionLifetime); + + bool succeeded = client.TryGetCurrent( + out ProcessSnapshot snapshot, + out CheatEngineFailure failure, + TestContext.Current.CancellationToken); + + Assert.False(succeeded); + Assert.Equal(default, snapshot); + Assert.Equal(CheatEngineFailureKind.InvalidHostResult, failure.Kind); + Assert.Equal("Processes.GetCurrent", failure.Operation); + Assert.IsType(failure.Exception); + Assert.Equal(0, selectionLifetime.Epoch); + } + + [Fact] + public void TryGetCurrentRethrowsUnexpectedHostExceptions() + { + FakeProcessHost host = FakeProcessHost.CreateSelected(42, CheatEngineArchitecture.X64); + ObjectDisposedException expected = new("fixture process host"); + host.GetOpenedProcessIdException = expected; + using ControlledCoreLifetimeContext activationContext = new(); + using CoreLifetime activationLifetime = new(activationContext); + using TargetSelectionLifetime selectionLifetime = CreateSelectionLifetime(); + ProcessClient client = new( + new SdkMainThreadDispatcher(activationLifetime, new InlineMainThreadInvoker()), + host, + selectionLifetime); + + ObjectDisposedException actual = Assert.Throws(() => + client.TryGetCurrent(out _, out _, TestContext.Current.CancellationToken)); + + Assert.Same(expected, actual); + } + + [Fact] + public void TryGetCurrentHonorsCancellationBeforeProductionDispatchAdmission() + { + FakeProcessHost host = FakeProcessHost.CreateSelected(42, CheatEngineArchitecture.X64); + using ControlledCoreLifetimeContext activationContext = new(); + using CoreLifetime activationLifetime = new(activationContext); + using TargetSelectionLifetime selectionLifetime = CreateSelectionLifetime(); + ProcessClient client = new( + new SdkMainThreadDispatcher(activationLifetime, new InlineMainThreadInvoker()), + host, + selectionLifetime); + using CancellationTokenSource cancellation = new(); + cancellation.Cancel(); + + bool succeeded = client.TryGetCurrent(out ProcessSnapshot snapshot, out CheatEngineFailure failure, + cancellation.Token); + + Assert.False(succeeded); + Assert.Equal(default, snapshot); + Assert.Equal(CheatEngineFailureKind.Cancelled, failure.Kind); + Assert.Equal(0, host.GetOpenedProcessIdCalls); + } + + [Fact] + public void InlineDispatcherPreservesCallbackExceptionIdentity() + { + InlineDispatcher dispatcher = new(); + InvalidOperationException expected = new("fixture callback"); + + InvalidOperationException actual = Assert.Throws(() => + dispatcher.TryInvoke(() => throw expected, out _, TestContext.Current.CancellationToken)); + + Assert.Same(expected, actual); + } + [Fact] public void AttachVerifiesThePidSelectedByCheatEngineBeforeReturningTheSnapshot() { FakeProcessHost host = FakeProcessHost.CreateSelected(42, CheatEngineArchitecture.X64); host.LocalProcesses[43] = new LocalProcessInfo(43, "fixture-b", "C:\\fixtures\\fixture-b.exe"); host.SelectedAfterOpenOverride = 42; + using ControlledCoreLifetimeContext activationContext = new(); + using CoreLifetime activationLifetime = new(activationContext); using TargetSelectionLifetime selectionLifetime = CreateSelectionLifetime(); - ProcessClient client = new(new InlineDispatcher(), host, selectionLifetime); + ProcessClient client = new( + new SdkMainThreadDispatcher(activationLifetime, new InlineMainThreadInvoker()), + host, + selectionLifetime); bool succeeded = client.TryAttach( new TargetProcessId(43), @@ -187,9 +319,42 @@ public void AttachVerifiesThePidSelectedByCheatEngineBeforeReturningTheSnapshot( Assert.False(succeeded); Assert.Equal(default, snapshot); Assert.Equal(CheatEngineFailureKind.OperationRejected, failure.Kind); + Assert.Equal("Processes.Attach", failure.Operation); Assert.Equal([43L], host.OpenProcessCalls); } + [Fact] + public void TryAttachReportsTargetNotAttachedWhenTheSelectedProcessDisappearsAfterOpen() + { + FakeProcessHost host = FakeProcessHost.CreateSelected(42, CheatEngineArchitecture.X64); + host.LocalProcesses[43] = new LocalProcessInfo(43, "fixture-b", "C:\\fixtures\\fixture-b.exe"); + host.AfterOpenProcess = static processHost => processHost.LocalProcesses.Remove(43); + using ControlledCoreLifetimeContext activationContext = new(); + using CoreLifetime activationLifetime = new(activationContext); + using TargetSelectionLifetime selectionLifetime = CreateSelectionLifetime(); + ProcessClient client = new( + new SdkMainThreadDispatcher(activationLifetime, new InlineMainThreadInvoker()), + host, + selectionLifetime); + ProcessSnapshot initial = client.GetCurrent(TestContext.Current.CancellationToken); + RecordingDisposable lease = new(); + selectionLifetime.Track(lease, initial.SelectionEpoch); + + bool succeeded = client.TryAttach( + new TargetProcessId(43), + out ProcessSnapshot snapshot, + out CheatEngineFailure failure, + TestContext.Current.CancellationToken); + + Assert.False(succeeded); + Assert.Equal(default, snapshot); + Assert.Equal(CheatEngineFailureKind.TargetNotAttached, failure.Kind); + Assert.Equal("Processes.Attach", failure.Operation); + Assert.Equal([43L], host.OpenProcessCalls); + Assert.Equal(1, selectionLifetime.Epoch); + Assert.Equal(1, lease.DisposeCount); + } + [Fact] public void TryAttachSelectsTheRequestedPidAndAdvancesAnAlreadyObservedSelection() { @@ -468,12 +633,30 @@ internal int GetLocalProcessesCalls private set; } + internal int GetOpenedProcessIdCalls + { + get; + private set; + } + internal Exception? GetLocalProcessesException { get; set; } + internal Exception? GetOpenedProcessIdException + { + get; + set; + } + + internal Action? AfterOpenProcess + { + get; + set; + } + internal long OpenedProcessId { get; @@ -494,6 +677,12 @@ internal CheatEngineArchitecture TargetArchitecture public long GetOpenedProcessId() { + GetOpenedProcessIdCalls++; + if (GetOpenedProcessIdException is { } exception) + { + throw exception; + } + return OpenedProcessId; } @@ -501,6 +690,7 @@ public void OpenProcess(long processId) { OpenProcessCalls.Add(processId); OpenedProcessId = SelectedAfterOpenOverride ?? processId; + AfterOpenProcess?.Invoke(this); } public bool TryGetLocalProcess(int processId, out LocalProcessInfo process) @@ -557,21 +747,9 @@ public bool TryInvoke(Action callback, out CheatEngineFailure failure, return false; } - try - { - callback(); - failure = default; - return true; - } - catch (Exception exception) - { - failure = new CheatEngineFailure( - CheatEngineFailureKind.OperationRejected, - "Dispatcher.Invoke", - exception.Message, - exception); - return false; - } + callback(); + failure = default; + return true; } public bool TryInvoke(Func callback, out T result, out CheatEngineFailure failure, @@ -588,22 +766,9 @@ public bool TryInvoke(Func callback, out T result, out CheatEngineFailure return false; } - try - { - result = callback(); - failure = default; - return true; - } - catch (Exception exception) - { - result = default!; - failure = new CheatEngineFailure( - CheatEngineFailureKind.OperationRejected, - "Dispatcher.Invoke", - exception.Message, - exception); - return false; - } + result = callback(); + failure = default; + return true; } public void Invoke(Action callback, CancellationToken cancellationToken = default)