From 53b641c3c215677d81c1593288bc237e39241bd0 Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 7 Oct 2026 05:30:20 +0900 Subject: [PATCH 1/6] Add a registry that ends each command activity exactly once macOS throttles a background Editor, and the fix holds an operating-system activity while a command runs. Ending the same native token twice touches released memory, so one registry owns the live tokens: each is ended by its handle or by the close before a domain reload, whichever comes first, and no activity starts after the close. A platform without the native entry points runs commands without an activity instead of failing them, and a failure while closing is logged so the other reload subscribers still run. --- .../Editor/EditorExecutionActivityTests.cs | 278 ++++++++++++++++++ .../EditorExecutionActivityTests.cs.meta | 11 + .../Infrastructure/ExecutionActivity.meta | 8 + .../EditorExecutionActivity.cs | 168 +++++++++++ .../EditorExecutionActivity.cs.meta | 11 + .../ExecutionActivity/IProcessActivityApi.cs | 27 ++ .../IProcessActivityApi.cs.meta | 11 + .../InertProcessActivityApi.cs | 25 ++ .../InertProcessActivityApi.cs.meta | 11 + 9 files changed, 550 insertions(+) create mode 100644 Assets/Tests/Editor/EditorExecutionActivityTests.cs create mode 100644 Assets/Tests/Editor/EditorExecutionActivityTests.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity.meta create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs.meta diff --git a/Assets/Tests/Editor/EditorExecutionActivityTests.cs b/Assets/Tests/Editor/EditorExecutionActivityTests.cs new file mode 100644 index 0000000000..aa2bd2d2e3 --- /dev/null +++ b/Assets/Tests/Editor/EditorExecutionActivityTests.cs @@ -0,0 +1,278 @@ +using System; +using System.Collections.Generic; +using System.Text.RegularExpressions; +using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the registry that holds an operating-system activity while a command runs: every activity it + /// starts ends exactly once, it starts none after closing, and commands still run when none can start. + /// + public sealed class EditorExecutionActivityTests + { + private const string MissingEntryPointMessage = "native entry point missing in this test"; + private const string EndFailureMessage = "End failed in this test"; + + /// + /// Verifies one hold starts one activity and disposing the hold ends that same token. + /// + [Test] + public void Hold_ThenDispose_BeginsOneActivityAndEndsThatToken() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + + IDisposable hold = activity.Hold(); + + Assert.That(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.LiveCount, Is.EqualTo(1)); + + hold.Dispose(); + + Assert.That(api.EndedTokens, Is.EqualTo(new[] { new IntPtr(1) })); + Assert.That(api.LiveCount, Is.EqualTo(0)); + } + + /// + /// Verifies disposing the same hold twice ends its activity only once. + /// + [Test] + public void Dispose_CalledTwice_EndsTheActivityOnce() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + IDisposable hold = activity.Hold(); + + hold.Dispose(); + hold.Dispose(); + + Assert.That(api.EndedTokens, Is.EqualTo(new[] { new IntPtr(1) })); + } + + /// + /// Verifies two overlapping holds each end their own token once, whichever is released first. + /// + [TestCase(0, 1)] + [TestCase(1, 0)] + public void Hold_TwiceOverlapping_EndsEachTokenOnceInEitherOrder(int releasedFirst, int releasedSecond) + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + IDisposable[] holds = { activity.Hold(), activity.Hold() }; + + Assert.That(api.LiveCount, Is.EqualTo(2)); + + holds[releasedFirst].Dispose(); + holds[releasedSecond].Dispose(); + + // Tokens count up from 1, so the hold at index i owns token i + 1. + Assert.That( + api.EndedTokens, + Is.EqualTo(new[] { new IntPtr(releasedFirst + 1), new IntPtr(releasedSecond + 1) })); + Assert.That(api.LiveCount, Is.EqualTo(0)); + } + + /// + /// Verifies closing ends every live activity, and disposing those holds afterwards ends nothing more. + /// + [Test] + public void ReleaseAllAndClose_EndsEveryLiveActivity_AndALaterDisposeEndsNothing() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + IDisposable first = activity.Hold(); + IDisposable second = activity.Hold(); + + activity.ReleaseAllAndClose(); + + Assert.That(api.EndedTokens, Is.EquivalentTo(new[] { new IntPtr(1), new IntPtr(2) })); + Assert.That(api.LiveCount, Is.EqualTo(0)); + + first.Dispose(); + second.Dispose(); + + Assert.That(api.EndedTokens.Count, Is.EqualTo(2)); + } + + /// + /// Verifies a hold requested after the registry closed starts no activity, while one requested + /// before closing did. + /// + [Test] + public void Hold_AfterReleaseAllAndClose_DoesNotBeginAnotherActivity() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + IDisposable earlyHold = activity.Hold(); + + Assert.That(api.BeginCount, Is.EqualTo(1)); + + activity.ReleaseAllAndClose(); + IDisposable lateHold = activity.Hold(); + + Assert.That(api.BeginCount, Is.EqualTo(1)); + + lateHold.Dispose(); + earlyHold.Dispose(); + } + + /// + /// Verifies a platform without the native entry points still lets commands run: the hold does not + /// throw, one warning is logged, and later holds do not try the native call again. + /// + [TestCase(typeof(EntryPointNotFoundException))] + [TestCase(typeof(DllNotFoundException))] + public void Hold_WhenThePlatformLacksTheNativeEntryPoints_RunsWithoutAnActivity_AndDoesNotTryAgain( + Type exceptionType) + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + api.BeginException = (Exception)Activator.CreateInstance(exceptionType, MissingEntryPointMessage); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + LogAssert.Expect(LogType.Warning, new Regex("throttling.*" + MissingEntryPointMessage)); + + IDisposable first = activity.Hold(); + IDisposable second = activity.Hold(); + + Assert.That(first, Is.Not.Null); + Assert.That(second, Is.Not.Null); + + first.Dispose(); + second.Dispose(); + + Assert.That(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.EndedTokens, Is.Empty); + } + + /// + /// Verifies a failing End reaches the caller and is never repeated for the same token, neither by + /// a second dispose nor by closing the registry. + /// + [Test] + public void Dispose_WhenEndThrows_DoesNotEndTheSameTokenAgain() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + IDisposable hold = activity.Hold(); + api.ThrowOnNextEnd = true; + + Assert.Throws(() => hold.Dispose()); + + hold.Dispose(); + activity.ReleaseAllAndClose(); + + Assert.That(api.EndedTokens, Is.EqualTo(new[] { new IntPtr(1) })); + } + + /// + /// Verifies a failing End while closing is logged instead of thrown, the other token is still ended, + /// and disposing the holds afterwards ends no token a second time. + /// + [Test] + public void ReleaseAllAndClose_WhenEndThrows_StillEndsTheOthers_AndEndsNoTokenTwice() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + IDisposable first = activity.Hold(); + IDisposable second = activity.Hold(); + api.ThrowOnNextEnd = true; + LogAssert.Expect(LogType.Exception, new Regex(EndFailureMessage)); + + Assert.DoesNotThrow(() => activity.ReleaseAllAndClose()); + Assert.That(api.EndedTokens, Is.EquivalentTo(new[] { new IntPtr(1), new IntPtr(2) })); + + first.Dispose(); + second.Dispose(); + + Assert.That(api.EndedTokens.Count, Is.EqualTo(2)); + } + + /// + /// Verifies that when the platform starts no activity, the hold is still a usable handle and + /// disposing it ends nothing. + /// + [Test] + public void Hold_WhenThePlatformReturnsNoToken_ReturnsAHandleThatEndsNothing() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + api.ReturnsNoToken = true; + EditorExecutionActivity activity = new EditorExecutionActivity(api); + + IDisposable hold = activity.Hold(); + + Assert.That(hold, Is.Not.Null); + + hold.Dispose(); + + Assert.That(api.EndedTokens, Is.Empty); + } + + /// + /// Verifies the API used off macOS never starts an activity. + /// + [Test] + public void InertProcessActivityApi_Begin_ReturnsNoToken() + { + InertProcessActivityApi api = new InertProcessActivityApi(); + + Assert.That(api.Begin("test reason"), Is.EqualTo(IntPtr.Zero)); + } + + /// + /// Records Begin and End calls in place of the operating system. Tokens count up from 1. + /// + private sealed class RecordingProcessActivityApi : IProcessActivityApi + { + private readonly HashSet _liveTokens = new HashSet(); + private readonly List _endedTokens = new List(); + private int _lastToken; + + public int BeginCount { get; private set; } + + public IReadOnlyList EndedTokens => _endedTokens; + + public int LiveCount => _liveTokens.Count; + + public bool ReturnsNoToken { get; set; } + + public Exception BeginException { get; set; } + + public bool ThrowOnNextEnd { get; set; } + + public IntPtr Begin(string reason) + { + BeginCount++; + if (BeginException != null) + { + throw BeginException; + } + + if (ReturnsNoToken) + { + return IntPtr.Zero; + } + + _lastToken++; + IntPtr token = new IntPtr(_lastToken); + _liveTokens.Add(token); + return token; + } + + public void End(IntPtr token) + { + _endedTokens.Add(token); + if (ThrowOnNextEnd) + { + ThrowOnNextEnd = false; + throw new InvalidOperationException(EndFailureMessage); + } + + _liveTokens.Remove(token); + } + } + } +} diff --git a/Assets/Tests/Editor/EditorExecutionActivityTests.cs.meta b/Assets/Tests/Editor/EditorExecutionActivityTests.cs.meta new file mode 100644 index 0000000000..7db0860ac0 --- /dev/null +++ b/Assets/Tests/Editor/EditorExecutionActivityTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2a8aa8af855b6460eb3541097fe81809 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity.meta b/Packages/src/Editor/Infrastructure/ExecutionActivity.meta new file mode 100644 index 0000000000..d4a2c29eea --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: b6110c72271644717b453c1d9932f269 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs new file mode 100644 index 0000000000..ff9d477cf6 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs @@ -0,0 +1,168 @@ +using System; +using System.Collections.Generic; +using UnityEditor; + +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Keeps the operating system from throttling the Editor while it runs a command. + /// Each hold starts one activity, and that activity ends exactly once: when the hold is disposed, + /// or when the registry closes before a domain reload, whichever comes first. + /// + internal sealed class EditorExecutionActivity + { + private const string ActivityReason = "Unity CLI Loop is executing a command"; + + private readonly IProcessActivityApi _api; + private readonly object _gate = new object(); + + // Why one set of live tokens: it is the single record of which token is still owed an End, so a + // second Dispose, or a Dispose after closing, finds nothing and cannot end a token twice. Ending a + // token twice touches released native memory and brings the Editor down. + private readonly HashSet _liveTokens = new HashSet(); + private bool _closed; + private bool _unavailable; + + internal EditorExecutionActivity(IProcessActivityApi api) + { + System.Diagnostics.Debug.Assert(api != null, "api must not be null"); + + _api = api ?? throw new ArgumentNullException(nameof(api)); + } + + /// + /// Creates the registry for this Editor domain. It closes itself before the next domain reload, + /// because a reload abandons awaiting commands without disposing their holds, and the native + /// activity would outlive the domain that started it. + /// + internal static EditorExecutionActivity CreateForEditor() + { + IProcessActivityApi api = new InertProcessActivityApi(); + EditorExecutionActivity activity = new EditorExecutionActivity(api); + AssemblyReloadEvents.beforeAssemblyReload += activity.ReleaseAllAndClose; + return activity; + } + + /// + /// Starts holding an activity for one command and returns the handle that ends it. + /// Any thread may call it. The handle holds nothing when the registry is closed or the platform + /// starts no activity; disposing it is still required and still safe. + /// + internal IDisposable Hold() + { + // Why Begin runs inside the lock: a close that races with this call must either see the new + // token in the set and end it, or have closed before Begin, so no token escapes the set. + lock (_gate) + { + if (_closed || _unavailable) + { + return new InertHold(); + } + + IntPtr token; + try + { + token = _api.Begin(ActivityReason); + } + catch (Exception exception) when (exception is DllNotFoundException || exception is EntryPointNotFoundException) + { + // Why only these two are caught: they mean the platform lacks the native entry points, + // and the activity only keeps a command fast, so a missing API must cost speed, never the + // command. Any other exception is a bug in the adapter and reaches the caller. + _unavailable = true; + UnityEngine.Debug.LogWarning( + $"[{UnityCliLoopConstants.PROJECT_NAME}] Commands may run slower while the Editor is in the background until the next domain reload: the activity that stops the operating system from throttling the Editor is unavailable ({exception.Message})."); + return new InertHold(); + } + + if (token == IntPtr.Zero) + { + return new InertHold(); + } + + _liveTokens.Add(token); + return new LiveHold(this, token); + } + } + + /// + /// Ends every live activity and stops starting new ones. It runs before a domain reload; + /// calling it again does nothing. + /// + internal void ReleaseAllAndClose() + { + lock (_gate) + { + _closed = true; + IntPtr[] tokens = new IntPtr[_liveTokens.Count]; + _liveTokens.CopyTo(tokens); + // Why clear before ending: every token handed to End has already left the set, so a hold + // disposed later finds nothing and cannot end it a second time. + _liveTokens.Clear(); + foreach (IntPtr token in tokens) + { + // Why a failure is logged instead of thrown: Unity runs every beforeAssemblyReload + // subscriber from one loop with no isolation, so an exception here would skip the + // subscribers after this one, including the one that stops the server before the + // domain is unloaded. The domain is about to be discarded, so the log is the only + // place left to report it, and the remaining tokens are still ended. + try + { + _api.End(token); + } + catch (Exception exception) + { + UnityEngine.Debug.LogException(exception); + } + } + } + } + + private void Release(IntPtr token) + { + lock (_gate) + { + // Why remove before End: the token must leave the set even when End throws, so neither a + // second Dispose nor a later close can end it again. + if (!_liveTokens.Remove(token)) + { + return; + } + + _api.End(token); + } + } + + /// + /// Handle for an activity this registry started; disposing it ends the activity once. + /// + private sealed class LiveHold : IDisposable + { + private readonly EditorExecutionActivity _owner; + private readonly IntPtr _token; + + internal LiveHold(EditorExecutionActivity owner, IntPtr token) + { + _owner = owner; + _token = token; + } + + public void Dispose() + { + _owner.Release(_token); + } + } + + /// + /// Handle returned when no activity was started; disposing it does nothing. + /// + private sealed class InertHold : IDisposable + { + public void Dispose() + { + } + } + } +} diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs.meta b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs.meta new file mode 100644 index 0000000000..29c66e22e0 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5b7d3ce67ac2748daa7bfd98bece2c38 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs b/Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs new file mode 100644 index 0000000000..662af3b9bd --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs @@ -0,0 +1,27 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Starts and ends an operating-system activity that keeps the Editor process from being throttled + /// while it works in the background. + /// + internal interface IProcessActivityApi + { + /// + /// Starts an activity and returns its token, or IntPtr.Zero when no activity was started + /// (the platform has no such activity, or the operating system declined). + /// A platform that lacks the native entry points may throw DllNotFoundException or + /// EntryPointNotFoundException; callers treat that as "unavailable for this domain". + /// + /// Non-empty ASCII text the operating system records with the activity. + IntPtr Begin(string reason); + + /// + /// Ends the activity of a token returned by Begin. Each non-zero token must be passed exactly once, + /// because ending the same token twice touches released native memory. + /// + /// A non-zero token returned by Begin. IntPtr.Zero throws ArgumentException. + void End(IntPtr token); + } +} diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs.meta b/Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs.meta new file mode 100644 index 0000000000..05133a7fa3 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/IProcessActivityApi.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: e00ee475d4a93410a9864e54906ddda3 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs b/Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs new file mode 100644 index 0000000000..3cf4aa017d --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs @@ -0,0 +1,25 @@ +using System; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Process activity API for platforms where the Editor has no background throttle to lift. + /// It never starts an activity. + /// + internal sealed class InertProcessActivityApi : IProcessActivityApi + { + public IntPtr Begin(string reason) + { + System.Diagnostics.Debug.Assert(!string.IsNullOrEmpty(reason), "reason must not be empty"); + + return IntPtr.Zero; + } + + public void End(IntPtr token) + { + // Why every call throws: Begin never hands out a token here, so no argument can be one + // this instance started, and a call means the caller broke the End contract. + throw new ArgumentException("This platform never starts an activity, so there is no token to end.", nameof(token)); + } + } +} diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs.meta b/Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs.meta new file mode 100644 index 0000000000..85c896f6fc --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/InertProcessActivityApi.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b24716c52c16145e3964d8a52aa0be2b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 520191df1803a21523f106f1e2ed70707a22adfb Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 7 Oct 2026 05:37:03 +0900 Subject: [PATCH 2/6] Hold an activity while the router runs a command or tool A request that reaches a throttled background Editor on macOS is served several times slower, so the router now holds an activity from the moment it dispatches a tool or internal bridge command until that call returns, throws or is canceled. The editor status answer stays outside the hold because it must answer while the main thread is stuck. The registry is created once per domain in the composition root and handed to every router the server factory builds; the test double moves to its own file so the router tests can watch the hold from inside a tool. --- .../Editor/EditorExecutionActivityTests.cs | 57 +---- ...nRpcRequestProcessorCliVersionGateTests.cs | 2 +- .../Editor/JsonRpcRequestProcessorTests.cs | 2 +- ...seFactoryWireShapeCharacterizationTests.cs | 2 +- .../Editor/RecordingProcessActivityApi.cs | 63 ++++++ .../RecordingProcessActivityApi.cs.meta | 11 + ...yCliLoopBridgeClientSessionManagerTests.cs | 2 +- ...CliLoopBridgeServerInstanceFactoryTests.cs | 3 +- .../UnityCliLoopBridgeServerLoopTests.cs | 2 +- ...nityCliLoopExecutionRouterActivityTests.cs | 200 ++++++++++++++++++ ...liLoopExecutionRouterActivityTests.cs.meta | 11 + .../Editor/UnityCliLoopToolRegistryTests.cs | 4 +- .../UnityCliLoopApplicationRegistration.cs | 4 +- .../Api/UnityCliLoopExecutionRouter.cs | 26 ++- ...UnityCliLoopBridgeServerInstanceFactory.cs | 9 +- 15 files changed, 330 insertions(+), 68 deletions(-) create mode 100644 Assets/Tests/Editor/RecordingProcessActivityApi.cs create mode 100644 Assets/Tests/Editor/RecordingProcessActivityApi.cs.meta create mode 100644 Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs create mode 100644 Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs.meta diff --git a/Assets/Tests/Editor/EditorExecutionActivityTests.cs b/Assets/Tests/Editor/EditorExecutionActivityTests.cs index aa2bd2d2e3..5a11e3a027 100644 --- a/Assets/Tests/Editor/EditorExecutionActivityTests.cs +++ b/Assets/Tests/Editor/EditorExecutionActivityTests.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Text.RegularExpressions; using NUnit.Framework; using UnityEngine; @@ -16,7 +15,6 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor public sealed class EditorExecutionActivityTests { private const string MissingEntryPointMessage = "native entry point missing in this test"; - private const string EndFailureMessage = "End failed in this test"; /// /// Verifies one hold starts one activity and disposing the hold ends that same token. @@ -180,7 +178,7 @@ public void ReleaseAllAndClose_WhenEndThrows_StillEndsTheOthers_AndEndsNoTokenTw IDisposable first = activity.Hold(); IDisposable second = activity.Hold(); api.ThrowOnNextEnd = true; - LogAssert.Expect(LogType.Exception, new Regex(EndFailureMessage)); + LogAssert.Expect(LogType.Exception, new Regex(RecordingProcessActivityApi.EndFailureMessage)); Assert.DoesNotThrow(() => activity.ReleaseAllAndClose()); Assert.That(api.EndedTokens, Is.EquivalentTo(new[] { new IntPtr(1), new IntPtr(2) })); @@ -221,58 +219,5 @@ public void InertProcessActivityApi_Begin_ReturnsNoToken() Assert.That(api.Begin("test reason"), Is.EqualTo(IntPtr.Zero)); } - - /// - /// Records Begin and End calls in place of the operating system. Tokens count up from 1. - /// - private sealed class RecordingProcessActivityApi : IProcessActivityApi - { - private readonly HashSet _liveTokens = new HashSet(); - private readonly List _endedTokens = new List(); - private int _lastToken; - - public int BeginCount { get; private set; } - - public IReadOnlyList EndedTokens => _endedTokens; - - public int LiveCount => _liveTokens.Count; - - public bool ReturnsNoToken { get; set; } - - public Exception BeginException { get; set; } - - public bool ThrowOnNextEnd { get; set; } - - public IntPtr Begin(string reason) - { - BeginCount++; - if (BeginException != null) - { - throw BeginException; - } - - if (ReturnsNoToken) - { - return IntPtr.Zero; - } - - _lastToken++; - IntPtr token = new IntPtr(_lastToken); - _liveTokens.Add(token); - return token; - } - - public void End(IntPtr token) - { - _endedTokens.Add(token); - if (ThrowOnNextEnd) - { - ThrowOnNextEnd = false; - throw new InvalidOperationException(EndFailureMessage); - } - - _liveTokens.Remove(token); - } - } } } diff --git a/Assets/Tests/Editor/JsonRpcRequestProcessorCliVersionGateTests.cs b/Assets/Tests/Editor/JsonRpcRequestProcessorCliVersionGateTests.cs index f313be7729..699daad302 100644 --- a/Assets/Tests/Editor/JsonRpcRequestProcessorCliVersionGateTests.cs +++ b/Assets/Tests/Editor/JsonRpcRequestProcessorCliVersionGateTests.cs @@ -658,7 +658,7 @@ private static UnityCliLoopToolRegistrarService CreateRegistrarServiceWithToolSe private static JsonRpcRequestProcessor CreateProcessor(UnityCliLoopToolRegistrarService service) { - UnityCliLoopExecutionRouter executionRouter = new(service); + UnityCliLoopExecutionRouter executionRouter = new(service, new EditorExecutionActivity(new InertProcessActivityApi())); return new JsonRpcRequestProcessor(executionRouter); } diff --git a/Assets/Tests/Editor/JsonRpcRequestProcessorTests.cs b/Assets/Tests/Editor/JsonRpcRequestProcessorTests.cs index 56a2cc99be..d366e89170 100644 --- a/Assets/Tests/Editor/JsonRpcRequestProcessorTests.cs +++ b/Assets/Tests/Editor/JsonRpcRequestProcessorTests.cs @@ -37,7 +37,7 @@ private static JsonRpcRequestProcessor CreateProcessor() new AllToolsEnabledSettingsPort(), new UnityCliLoopToolExecutionService(new IdleEditorRuntimeStatePort()), () => Array.Empty()); - return new JsonRpcRequestProcessor(new UnityCliLoopExecutionRouter(registrarService)); + return new JsonRpcRequestProcessor(new UnityCliLoopExecutionRouter(registrarService, new EditorExecutionActivity(new InertProcessActivityApi()))); } private sealed class AllToolsEnabledSettingsPort : IToolSettingsPort diff --git a/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs b/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs index cc2f54d272..705ea27ebf 100644 --- a/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs +++ b/Assets/Tests/Editor/JsonRpcResponseFactoryWireShapeCharacterizationTests.cs @@ -171,7 +171,7 @@ public async Task ProcessRequest_WhenProtocolVersionIsTooOld_ProducesFrozenMisma private static JsonRpcRequestProcessor CreateProcessor(UnityCliLoopToolRegistrarService service) { - UnityCliLoopExecutionRouter executionRouter = new(service); + UnityCliLoopExecutionRouter executionRouter = new(service, new EditorExecutionActivity(new InertProcessActivityApi())); return new JsonRpcRequestProcessor(executionRouter); } diff --git a/Assets/Tests/Editor/RecordingProcessActivityApi.cs b/Assets/Tests/Editor/RecordingProcessActivityApi.cs new file mode 100644 index 0000000000..6575552567 --- /dev/null +++ b/Assets/Tests/Editor/RecordingProcessActivityApi.cs @@ -0,0 +1,63 @@ +using System; +using System.Collections.Generic; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test double for the operating-system activity calls. It records Begin and End calls, + /// hands out tokens that count up from 1, and can be told to fail either call. + /// + internal sealed class RecordingProcessActivityApi : IProcessActivityApi + { + internal const string EndFailureMessage = "End failed in this test"; + + private readonly HashSet _liveTokens = new HashSet(); + private readonly List _endedTokens = new List(); + private int _lastToken; + + internal int BeginCount { get; private set; } + + internal IReadOnlyList EndedTokens => _endedTokens; + + internal int LiveCount => _liveTokens.Count; + + internal bool ReturnsNoToken { get; set; } + + internal Exception BeginException { get; set; } + + internal bool ThrowOnNextEnd { get; set; } + + public IntPtr Begin(string reason) + { + BeginCount++; + if (BeginException != null) + { + throw BeginException; + } + + if (ReturnsNoToken) + { + return IntPtr.Zero; + } + + _lastToken++; + IntPtr token = new IntPtr(_lastToken); + _liveTokens.Add(token); + return token; + } + + public void End(IntPtr token) + { + _endedTokens.Add(token); + if (ThrowOnNextEnd) + { + ThrowOnNextEnd = false; + throw new InvalidOperationException(EndFailureMessage); + } + + _liveTokens.Remove(token); + } + } +} diff --git a/Assets/Tests/Editor/RecordingProcessActivityApi.cs.meta b/Assets/Tests/Editor/RecordingProcessActivityApi.cs.meta new file mode 100644 index 0000000000..4d62d01d47 --- /dev/null +++ b/Assets/Tests/Editor/RecordingProcessActivityApi.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ffe55ef9ccf904e3c82cc37c1ff5520c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCliLoopBridgeClientSessionManagerTests.cs b/Assets/Tests/Editor/UnityCliLoopBridgeClientSessionManagerTests.cs index 0c64b7321f..60e3508b81 100644 --- a/Assets/Tests/Editor/UnityCliLoopBridgeClientSessionManagerTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopBridgeClientSessionManagerTests.cs @@ -145,7 +145,7 @@ private static UnityCliLoopBridgeClientSessionManager CreateManager() new AllToolsEnabledSettingsPort(), new UnityCliLoopToolExecutionService(new IdleEditorRuntimeStatePort()), () => Array.Empty()); - JsonRpcRequestProcessor processor = new JsonRpcRequestProcessor(new UnityCliLoopExecutionRouter(registrarService)); + JsonRpcRequestProcessor processor = new JsonRpcRequestProcessor(new UnityCliLoopExecutionRouter(registrarService, new EditorExecutionActivity(new InertProcessActivityApi()))); return new UnityCliLoopBridgeClientSessionManager( processor, new UnityCliLoopBridgeHeartbeatService(), diff --git a/Assets/Tests/Editor/UnityCliLoopBridgeServerInstanceFactoryTests.cs b/Assets/Tests/Editor/UnityCliLoopBridgeServerInstanceFactoryTests.cs index c6a6e54047..d4afb069eb 100644 --- a/Assets/Tests/Editor/UnityCliLoopBridgeServerInstanceFactoryTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopBridgeServerInstanceFactoryTests.cs @@ -21,7 +21,8 @@ public void Create_WhenCalledTwice_ReturnsDistinctStoppedBridgeServers() { UnityCliLoopBridgeServerInstanceFactory factory = new UnityCliLoopBridgeServerInstanceFactory( new NoOpDomainReloadDetectionService(), - CreateRegistrarService()); + CreateRegistrarService(), + new EditorExecutionActivity(new InertProcessActivityApi())); IUnityCliLoopServerInstance first = factory.Create(); IUnityCliLoopServerInstance second = factory.Create(); diff --git a/Assets/Tests/Editor/UnityCliLoopBridgeServerLoopTests.cs b/Assets/Tests/Editor/UnityCliLoopBridgeServerLoopTests.cs index ec6b8cd5d5..0812344c92 100644 --- a/Assets/Tests/Editor/UnityCliLoopBridgeServerLoopTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopBridgeServerLoopTests.cs @@ -354,7 +354,7 @@ private static UnityCliLoopBridgeServer CreateServer( () => Array.Empty()); return new UnityCliLoopBridgeServer( new NoOpDomainReloadDetectionService(), - new JsonRpcRequestProcessor(new UnityCliLoopExecutionRouter(registrarService)), + new JsonRpcRequestProcessor(new UnityCliLoopExecutionRouter(registrarService, new EditorExecutionActivity(new InertProcessActivityApi()))), new UnityCliLoopBridgeHeartbeatService(), new UnityCliLoopBridgeClientDisconnectMonitor(), createListener, diff --git a/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs b/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs new file mode 100644 index 0000000000..f5e893141d --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs @@ -0,0 +1,200 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the execution router holds an operating-system activity for exactly the length of each + /// tool or internal bridge command, and never for the editor status answer. + /// + public sealed class UnityCliLoopExecutionRouterActivityTests + { + /// + /// Verifies a tool runs while one activity is held, and the activity ends once the tool returns. + /// + [Test] + public async Task ExecuteAsync_Tool_HoldsAnActivityWhileTheToolRuns_AndReleasesItAfterwards() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + ActivityObservingTool tool = new ActivityObservingTool(api); + UnityCliLoopExecutionRouter router = CreateRouter(api, tool); + + UnityCliLoopToolResponse response = await router.ExecuteAsync( + ActivityObservingTool.Name, + new JObject(), + CancellationToken.None); + + Assert.That(response, Is.InstanceOf()); + Assert.That(tool.LiveCountWhileRunning, Is.EqualTo(1)); + Assert.That(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.LiveCount, Is.EqualTo(0)); + } + + /// + /// Verifies a tool that throws still releases its activity, and the caller sees the tool's exception. + /// + [Test] + public async Task ExecuteAsync_ToolThrows_ReleasesTheActivity_AndRethrows() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + UnityCliLoopExecutionRouter router = CreateRouter(api, new ThrowingTool()); + + Exception caught = null; + try + { + await router.ExecuteAsync(ThrowingTool.Name, new JObject(), CancellationToken.None); + } + catch (Exception exception) + { + caught = exception; + } + + Assert.That(caught, Is.InstanceOf()); + Assert.That(caught.Message, Is.EqualTo(ThrowingTool.FailureMessage)); + Assert.That(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.EndedTokens.Count, Is.EqualTo(1)); + Assert.That(api.LiveCount, Is.EqualTo(0)); + } + + /// + /// Verifies an internal bridge command canceled before it starts releases its activity, and the + /// caller still sees the cancellation. + /// + [Test] + public async Task ExecuteAsync_InternalCommandCanceledBeforeItStarts_ReleasesTheActivity() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + UnityCliLoopExecutionRouter router = CreateRouter(api); + + // Why the exception is caught into a variable: a test that ends Canceled is recorded as passed. + Exception caught = null; + try + { + await router.ExecuteAsync( + UnityCliLoopConstants.COMMAND_NAME_GET_VERSION, + new JObject(), + new CancellationToken(true)); + } + catch (Exception exception) + { + caught = exception; + } + + Assert.That(caught, Is.InstanceOf()); + Assert.That(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.EndedTokens.Count, Is.EqualTo(1)); + Assert.That(api.LiveCount, Is.EqualTo(0)); + } + + /// + /// Verifies the editor status answer starts no activity, because it must answer even while the + /// Editor main thread is blocked and so stays free of anything that can block or fail. + /// + [Test] + public async Task ExecuteAsync_EditorStatus_DoesNotBeginAnActivity() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + UnityCliLoopExecutionRouter router = CreateRouter(api); + + UnityCliLoopToolResponse response = await router.ExecuteAsync( + UnityCliLoopConstants.COMMAND_NAME_GET_EDITOR_STATUS, + new JObject(), + CancellationToken.None); + + Assert.That(response, Is.InstanceOf()); + Assert.That(api.BeginCount, Is.EqualTo(0)); + } + + /// + /// Verifies an internal bridge command holds one activity and ends it when the command returns. + /// + [Test] + public async Task ExecuteAsync_InternalCommand_HoldsAndReleasesAnActivity() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + UnityCliLoopExecutionRouter router = CreateRouter(api); + + UnityCliLoopToolResponse response = await router.ExecuteAsync( + UnityCliLoopConstants.COMMAND_NAME_GET_VERSION, + new JObject(), + CancellationToken.None); + + Assert.That(response, Is.InstanceOf()); + Assert.That(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.EndedTokens.Count, Is.EqualTo(1)); + Assert.That(api.LiveCount, Is.EqualTo(0)); + } + + private static UnityCliLoopExecutionRouter CreateRouter( + RecordingProcessActivityApi api, + params IUnityCliLoopTool[] tools) + { + UnityCliLoopToolRegistrarService registrarService = new UnityCliLoopToolRegistrarService( + new EmptyInternalToolNameProvider(), + new AlwaysEnabledToolSettingsPort(), + new UnityCliLoopToolExecutionService(new NoOpEditorRuntimeStatePort()), + () => tools); + return new UnityCliLoopExecutionRouter(registrarService, new EditorExecutionActivity(api)); + } + + /// + /// Tool that records how many activities were live while it ran. + /// + private sealed class ActivityObservingTool : IUnityCliLoopTool + { + public const string Name = "activity-observing-test"; + + private readonly RecordingProcessActivityApi _api; + + public ActivityObservingTool(RecordingProcessActivityApi api) + { + _api = api; + } + + public int? LiveCountWhileRunning { get; private set; } + + public string ToolName => Name; + + public ToolParameterSchema ParameterSchema => new ToolParameterSchema(); + + public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) + { + LiveCountWhileRunning = _api.LiveCount; + return Task.FromResult(new ActivityTestResponse()); + } + } + + /// + /// Tool whose execution fails. + /// + private sealed class ThrowingTool : IUnityCliLoopTool + { + public const string Name = "activity-throwing-test"; + public const string FailureMessage = "tool failed in this test"; + + public string ToolName => Name; + + public ToolParameterSchema ParameterSchema => new ToolParameterSchema(); + + public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) + { + return Task.FromException(new InvalidOperationException(FailureMessage)); + } + } + + /// + /// Response returned by the test tools. + /// + private sealed class ActivityTestResponse : UnityCliLoopToolResponse + { + } + } +} diff --git a/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs.meta b/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs.meta new file mode 100644 index 0000000000..1d944f40ad --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: e505759c5552746cc978f7d9c2536ea3 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs b/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs index 3fde8490e4..d6b0d3a171 100644 --- a/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopToolRegistryTests.cs @@ -561,7 +561,7 @@ private static UnityCliLoopExecutionRouter CreateExecutionRouter() { UnityCliLoopToolRegistrarService toolRegistrarService = UnityCliLoopToolRegistrarTestFactory.Create(UnityCliLoopToolDiscovery.DiscoverTools); - return new UnityCliLoopExecutionRouter(toolRegistrarService); + return new UnityCliLoopExecutionRouter(toolRegistrarService, new EditorExecutionActivity(new InertProcessActivityApi())); } private static UnityCliLoopExecutionRouter CreateExecutionRouterWithDevelopmentOnlyTool() @@ -569,7 +569,7 @@ private static UnityCliLoopExecutionRouter CreateExecutionRouterWithDevelopmentO UnityCliLoopToolRegistrarService toolRegistrarService = UnityCliLoopToolRegistrarTestFactory.Create(UnityCliLoopToolDiscovery.DiscoverTools); toolRegistrarService.RegisterCustomTool(new DevelopmentOnlyCatalogTestTool()); - return new UnityCliLoopExecutionRouter(toolRegistrarService); + return new UnityCliLoopExecutionRouter(toolRegistrarService, new EditorExecutionActivity(new InertProcessActivityApi())); } [Test] diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs index bb0aa25ae2..1e8b67a2bb 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs @@ -68,9 +68,11 @@ internal UnityCliLoopApplicationServices Register() new CliInstallationDetector(cliPinReaderService), new NativeCliInstallerService(), cliPinReaderService); + EditorExecutionActivity executionActivity = EditorExecutionActivity.CreateForEditor(); UnityCliLoopBridgeServerInstanceFactory serverFactory = new( domainReloadDetectionService, - toolRegistrarService); + toolRegistrarService, + executionActivity); UnityCliLoopServerLifecycleRegistryService lifecycleRegistry = new(); lifecycleRegistry.RegisterSource(serverFactory); UnityCliLoopServerStartupService serverStartupService = new( diff --git a/Packages/src/Editor/Infrastructure/Api/UnityCliLoopExecutionRouter.cs b/Packages/src/Editor/Infrastructure/Api/UnityCliLoopExecutionRouter.cs index 6a5beec28a..5402310054 100644 --- a/Packages/src/Editor/Infrastructure/Api/UnityCliLoopExecutionRouter.cs +++ b/Packages/src/Editor/Infrastructure/Api/UnityCliLoopExecutionRouter.cs @@ -19,13 +19,19 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure internal sealed class UnityCliLoopExecutionRouter { private readonly UnityCliLoopToolRegistrarService _toolRegistrarService; + private readonly EditorExecutionActivity _executionActivity; - internal UnityCliLoopExecutionRouter(UnityCliLoopToolRegistrarService toolRegistrarService) + internal UnityCliLoopExecutionRouter( + UnityCliLoopToolRegistrarService toolRegistrarService, + EditorExecutionActivity executionActivity) { System.Diagnostics.Debug.Assert(toolRegistrarService != null, "toolRegistrarService must not be null"); + System.Diagnostics.Debug.Assert(executionActivity != null, "executionActivity must not be null"); _toolRegistrarService = toolRegistrarService ?? throw new ArgumentNullException(nameof(toolRegistrarService)); + _executionActivity = executionActivity + ?? throw new ArgumentNullException(nameof(executionActivity)); } /// @@ -53,6 +59,24 @@ public async Task ExecuteAsync( EditorMainThreadLivenessTracker.SecondsSinceLastMainThreadTick()); } + // Why hold an activity for the length of the request: macOS throttles an Editor that is not + // frontmost, and a request that arrives while it is throttled is served several times slower. + // Why not around the status answer above: that path must stay free of anything that can block + // or fail, because it answers while the main thread is stuck. + using (_executionActivity.Hold()) + { + return await ExecuteCommandOrToolAsync(methodName, paramsToken, ct); + } + } + + /// + /// Runs one internal bridge command on the main thread, or hands one tool to the registrar. + /// + private async Task ExecuteCommandOrToolAsync( + string methodName, + JToken paramsToken, + CancellationToken ct) + { UnityCliLoopToolResponse response; if (InternalBridgeCommandRouter.IsInternalCommand(methodName)) { diff --git a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServerInstanceFactory.cs b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServerInstanceFactory.cs index 38319074d6..2a6a50b6c4 100644 --- a/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServerInstanceFactory.cs +++ b/Packages/src/Editor/Infrastructure/UnityCliLoopBridgeServerInstanceFactory.cs @@ -15,25 +15,30 @@ public sealed class UnityCliLoopBridgeServerInstanceFactory : public event Action ServerLoopExited; private readonly IDomainReloadDetectionService _domainReloadDetectionService; private readonly UnityCliLoopToolRegistrarService _toolRegistrarService; + private readonly EditorExecutionActivity _executionActivity; internal UnityCliLoopBridgeServerInstanceFactory( IDomainReloadDetectionService domainReloadDetectionService, - UnityCliLoopToolRegistrarService toolRegistrarService) + UnityCliLoopToolRegistrarService toolRegistrarService, + EditorExecutionActivity executionActivity) { System.Diagnostics.Debug.Assert(domainReloadDetectionService != null, "domainReloadDetectionService must not be null"); System.Diagnostics.Debug.Assert(toolRegistrarService != null, "toolRegistrarService must not be null"); + System.Diagnostics.Debug.Assert(executionActivity != null, "executionActivity must not be null"); _domainReloadDetectionService = domainReloadDetectionService ?? throw new ArgumentNullException(nameof(domainReloadDetectionService)); _toolRegistrarService = toolRegistrarService ?? throw new ArgumentNullException(nameof(toolRegistrarService)); + _executionActivity = executionActivity + ?? throw new ArgumentNullException(nameof(executionActivity)); } public IUnityCliLoopServerInstance Create() { UnityCliLoopBridgeHeartbeatService heartbeatService = new(); UnityCliLoopBridgeClientDisconnectMonitor clientDisconnectMonitor = new(); - UnityCliLoopExecutionRouter executionRouter = new(_toolRegistrarService); + UnityCliLoopExecutionRouter executionRouter = new(_toolRegistrarService, _executionActivity); JsonRpcRequestProcessor jsonRpcRequestProcessor = new(executionRouter); UnityCliLoopBridgeServer server = new( _domainReloadDetectionService, From 20ca395281600d979f844c751ff0b5558e4ab34b Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 7 Oct 2026 05:39:16 +0900 Subject: [PATCH 3/6] Hold the macOS activity through libobjc on macOS Editors The registry now uses NSProcessInfo's beginActivityWithOptions:reason: with NSActivityUserInitiatedAllowingIdleSystemSleep, the smallest option measured to lift App Nap, and leaves idle system sleep alone. Begin runs inside its own autorelease pool because thread-pool threads have none, and retains the token before the pool drains so the activity outlives the call. Other platforms keep the inert API, and the declarations resolve only when called, so no conditional compilation is needed. --- .../Editor/MacProcessActivityApiTests.cs | 49 ++++++++++ .../Editor/MacProcessActivityApiTests.cs.meta | 11 +++ .../EditorExecutionActivity.cs | 7 +- .../MacProcessActivityApi.cs | 92 +++++++++++++++++++ .../MacProcessActivityApi.cs.meta | 11 +++ 5 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 Assets/Tests/Editor/MacProcessActivityApiTests.cs create mode 100644 Assets/Tests/Editor/MacProcessActivityApiTests.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs create mode 100644 Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs.meta diff --git a/Assets/Tests/Editor/MacProcessActivityApiTests.cs b/Assets/Tests/Editor/MacProcessActivityApiTests.cs new file mode 100644 index 0000000000..3bced49bb4 --- /dev/null +++ b/Assets/Tests/Editor/MacProcessActivityApiTests.cs @@ -0,0 +1,49 @@ +using System; +using NUnit.Framework; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies the macOS process activity adapter against the real operating system calls. + /// + public sealed class MacProcessActivityApiTests + { + /// + /// Verifies that on macOS the real calls start an activity with a non-zero token and end it without + /// throwing, three times in a row. Other platforms skip the test. + /// + [Test] + public void BeginThenEnd_OnMacOS_ReturnsATokenAndDoesNotThrow() + { + // Why a runtime check instead of a platform attribute: this repository has no precedent for + // narrowing EditMode tests by platform attribute, and how it behaves there is unverified. + if (UnityEngine.Application.platform != RuntimePlatform.OSXEditor) + { + Assert.Ignore("macOS only"); + } + + MacProcessActivityApi api = new MacProcessActivityApi(); + for (int attempt = 0; attempt < 3; attempt++) + { + IntPtr token = api.Begin("Unity CLI Loop activity test"); + + Assert.That(token, Is.Not.EqualTo(IntPtr.Zero)); + Assert.DoesNotThrow(() => api.End(token)); + } + } + + /// + /// Verifies ending with no token is rejected before any native call, so it holds on every platform. + /// + [Test] + public void End_WithNoToken_Throws() + { + MacProcessActivityApi api = new MacProcessActivityApi(); + + Assert.Throws(() => api.End(IntPtr.Zero)); + } + } +} diff --git a/Assets/Tests/Editor/MacProcessActivityApiTests.cs.meta b/Assets/Tests/Editor/MacProcessActivityApiTests.cs.meta new file mode 100644 index 0000000000..2ae3a15e91 --- /dev/null +++ b/Assets/Tests/Editor/MacProcessActivityApiTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 51c4092e126984dc19c1228ca0e1ede6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs index ff9d477cf6..72baf4574a 100644 --- a/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs @@ -33,13 +33,16 @@ internal EditorExecutionActivity(IProcessActivityApi api) } /// - /// Creates the registry for this Editor domain. It closes itself before the next domain reload, + /// Creates the registry for this Editor domain: it holds the macOS activity on macOS and holds + /// nothing on other platforms. It closes itself before the next domain reload, /// because a reload abandons awaiting commands without disposing their holds, and the native /// activity would outlive the domain that started it. /// internal static EditorExecutionActivity CreateForEditor() { - IProcessActivityApi api = new InertProcessActivityApi(); + IProcessActivityApi api = UnityEngine.Application.platform == UnityEngine.RuntimePlatform.OSXEditor + ? new MacProcessActivityApi() + : new InertProcessActivityApi(); EditorExecutionActivity activity = new EditorExecutionActivity(api); AssemblyReloadEvents.beforeAssemblyReload += activity.ReleaseAllAndClose; return activity; diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs b/Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs new file mode 100644 index 0000000000..d8b19c3fe5 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs @@ -0,0 +1,92 @@ +using System; +using System.Runtime.InteropServices; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Starts and ends the macOS process activity (NSProcessInfo beginActivityWithOptions:reason:) that + /// keeps App Nap from throttling the Editor while it is in the background. + /// The declarations resolve only when called, so other platforms can load this type without harm. + /// + internal sealed class MacProcessActivityApi : IProcessActivityApi + { + private const string ObjCLibrary = "/usr/lib/libobjc.A.dylib"; + + // NSActivityUserInitiatedAllowingIdleSystemSleep in NSProcessInfo.h. Why this option: it was the + // smallest one measured to lift the throttle (NSActivityBackground and NSActivityLatencyCritical + // did not), and unlike NSActivityUserInitiated it leaves idle system sleep alone. + private const ulong UserInitiatedAllowingIdleSystemSleep = 0x00EFFFFFUL; + + public IntPtr Begin(string reason) + { + System.Diagnostics.Debug.Assert(!string.IsNullOrEmpty(reason), "reason must not be empty"); + + // Why a pool of our own: callers run on thread-pool threads, which have no autorelease pool, so + // the autoreleased token and reason string would never be freed. Draining this pool frees both, + // leaving only the reference retained below. + IntPtr pool = PushAutoreleasePool(); + try + { + IntPtr processInfo = Send(GetClass("NSProcessInfo"), RegisterSelector("processInfo")); + IntPtr reasonString = SendUtf8String( + GetClass("NSString"), + RegisterSelector("stringWithUTF8String:"), + reason); + IntPtr token = SendBeginActivity( + processInfo, + RegisterSelector("beginActivityWithOptions:reason:"), + UserInitiatedAllowingIdleSystemSleep, + reasonString); + // Why retain before the pool drains: the token is autoreleased, and draining the pool would + // free it and end the activity at once, while every call here still appears to succeed. + if (token != IntPtr.Zero) + { + Send(token, RegisterSelector("retain")); + } + + return token; + } + finally + { + PopAutoreleasePool(pool); + } + } + + public void End(IntPtr token) + { + if (token == IntPtr.Zero) + { + throw new ArgumentException("token must be a non-zero value returned by Begin.", nameof(token)); + } + + IntPtr processInfo = Send(GetClass("NSProcessInfo"), RegisterSelector("processInfo")); + SendEndActivity(processInfo, RegisterSelector("endActivity:"), token); + Send(token, RegisterSelector("release")); + } + + // objc_msgSend takes a different argument list per selector, so each shape gets its own name. + [DllImport(ObjCLibrary, EntryPoint = "objc_getClass")] + private static extern IntPtr GetClass(string name); + + [DllImport(ObjCLibrary, EntryPoint = "sel_registerName")] + private static extern IntPtr RegisterSelector(string name); + + [DllImport(ObjCLibrary, EntryPoint = "objc_msgSend")] + private static extern IntPtr Send(IntPtr receiver, IntPtr selector); + + [DllImport(ObjCLibrary, EntryPoint = "objc_msgSend")] + private static extern IntPtr SendUtf8String(IntPtr receiver, IntPtr selector, string value); + + [DllImport(ObjCLibrary, EntryPoint = "objc_msgSend")] + private static extern IntPtr SendBeginActivity(IntPtr receiver, IntPtr selector, ulong options, IntPtr reason); + + [DllImport(ObjCLibrary, EntryPoint = "objc_msgSend")] + private static extern void SendEndActivity(IntPtr receiver, IntPtr selector, IntPtr token); + + [DllImport(ObjCLibrary, EntryPoint = "objc_autoreleasePoolPush")] + private static extern IntPtr PushAutoreleasePool(); + + [DllImport(ObjCLibrary, EntryPoint = "objc_autoreleasePoolPop")] + private static extern void PopAutoreleasePool(IntPtr pool); + } +} diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs.meta b/Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs.meta new file mode 100644 index 0000000000..52c5f734e0 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/MacProcessActivityApi.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 47c0992b385d3484fa76513ec970f2d8 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From d4c3a8557d0b5f3c0a2b1d1766b6b2d43e74f264 Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 7 Oct 2026 05:59:29 +0900 Subject: [PATCH 4/6] Note that ending an activity token twice crashes the Editor A mutation test that let a hold end its token after the registry had already closed brought the Editor down with SIGTRAP inside endActivity:. The comments on the two places that end a token now say that a second end is a crash rather than an exception, so the remove-before-End order is not mistaken for defensive tidiness. --- .../ExecutionActivity/EditorExecutionActivity.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs index 72baf4574a..7d844f0909 100644 --- a/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs @@ -102,7 +102,9 @@ internal void ReleaseAllAndClose() IntPtr[] tokens = new IntPtr[_liveTokens.Count]; _liveTokens.CopyTo(tokens); // Why clear before ending: every token handed to End has already left the set, so a hold - // disposed later finds nothing and cannot end it a second time. + // disposed later finds nothing and cannot end it a second time. A second end is a crash, not + // an exception: endActivity: on an ended token stops the Editor with SIGTRAP. A mutation test + // brought the Editor down this way when a hold disposed after a reload's close ended its token. _liveTokens.Clear(); foreach (IntPtr token in tokens) { @@ -128,7 +130,8 @@ private void Release(IntPtr token) lock (_gate) { // Why remove before End: the token must leave the set even when End throws, so neither a - // second Dispose nor a later close can end it again. + // second Dispose nor a later close can end it again. Ending a token twice does not throw: the + // Editor stops with SIGTRAP, as a mutation test that skipped this check showed. if (!_liveTokens.Remove(token)) { return; From dc29bcc7a30d5635c3ad3619c2e31cdddaf5074f Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 7 Oct 2026 06:52:39 +0900 Subject: [PATCH 5/6] Record why the Editor holds a macOS activity only while a command runs The decision has alternatives that look simpler (holding for as long as the server runs, asking users to disable App Nap with defaults, other activity options), so the ADR keeps the measurements that ruled them out, what the hold does not cover, and when to reopen the question. --- ...d-a-macos-activity-while-a-command-runs.md | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 docs/adr/0012-hold-a-macos-activity-while-a-command-runs.md diff --git a/docs/adr/0012-hold-a-macos-activity-while-a-command-runs.md b/docs/adr/0012-hold-a-macos-activity-while-a-command-runs.md new file mode 100644 index 0000000000..65c3fc4622 --- /dev/null +++ b/docs/adr/0012-hold-a-macos-activity-while-a-command-runs.md @@ -0,0 +1,76 @@ +# Hold a macOS Activity Only While a Command Runs + +Date: 2026-10-07 + +## Decision + +While the Unity Editor processes a command — a tool or an internal bridge command — it holds a +macOS process activity started with `NSActivityUserInitiatedAllowingIdleSystemSleep`. The +activity begins when the execution router starts the command and ends when the router returns, +whether the command succeeded, failed, or was canceled. `get-editor-status` holds nothing: it +answers before the router starts any command, and it must keep answering while the main thread +is stuck. Every activity still held is ended before a domain reload. Other platforms hold +nothing. + +## Context + +macOS throttles an application that is not frontmost (App Nap). An Editor left in the +background enters this suppression within about a minute, and from then on everything it does +runs several times slower. An investigation on Unity 2022.3 measured this with a probe that read +the process's suppression state: + +- A CPU-only loop with no allocation took 48–62 ms unsuppressed and 59–660 ms suppressed, while + its thread CPU time stayed at 53–77 ms. No unsuppressed sample was slow. +- Hot reload of 200 methods took about 2.0–2.3 s unsuppressed and 16.7–20.2 s suppressed. +- Accepting a command — accept, router, start on the main thread — was barely delayed while + suppressed (a few ms to 43 ms on the Editor side). What slowed down was the processing after + it, so holding an activity only while a command is processed is enough. +- Beginning an activity with `NSActivityUserInitiatedAllowingIdleSystemSleep` (`0x00EFFFFF`) + from inside the Editor lifted the suppression within 2 ms even when it was already in place + (3 of 3). The suppression never came back while the activity was held (30 s three times, + 77 s once), and it came back 62–556 ms after `endActivity:`. +- Each compile left the Editor unsuppressed for about 34–55 s; what lifts it is unconfirmed. + +The token that `beginActivityWithOptions:reason:` returns is autoreleased, so the adapter +retains it before its own autorelease pool drains and releases it after `endActivity:`. + +## Rejected Alternatives + +- **`NSActivityBackground` or `NSActivityLatencyCritical`.** Measured: the suppression came back + while either was held. +- **Keep holding for a while after the last command.** It saves only the tens of milliseconds of + accepting the next command, and the hold would have to outlive domain reloads, which discard + every managed object. +- **Hold for as long as the server runs.** An Editor nobody is using would keep drawing power as + if it were in front. +- **Ask users to set `NSAppSleepDisabled` with `defaults`.** It changes the user's settings and + turns App Nap off for the whole Editor, not only while a command runs. +- **Bring the Editor to the front.** It takes focus away from whatever the user is doing. + +## Consequences + +- A long command, such as `run-tests`, holds the activity for its whole length. +- A command that never returns — a request waiting on a main thread that stays blocked, a tool + that never finishes — keeps its activity until the next domain reload or until the Editor + exits. This is intended: the hold ends with the command. +- Each activity token is ended exactly once. A second end does not throw: `endActivity:` on an + ended token crashes the Editor with SIGTRAP, as a mutation test of the registry showed. +- On a macOS whose native entry points are missing, the Editor logs one warning and runs + commands without the activity until the next domain reload. +- Work that continues after a command has responded is not covered. The commands known to leave + such work are `compile` (it responds when the compile finishes; the domain reload follows), + `control-play-mode` (entering or leaving Play Mode and the reload that comes with it), + `execute-dynamic-code` when it waits for a domain reload (the compile and the reload), + `run-tests` in Play Mode (the run continues in the new domain after the reload cuts off the + request), `record-video`, `replay-input`, and `enable-watch` (they register per-frame work and + return), and the internal bridge command `set-code-optimization-debug` (the recompile and + reload after the switch). +- Windows and Linux have not been investigated; they hold nothing. +- A Begin and End pair costs about 4 µs inside the Editor (median of 100 pairs; the slowest took + 0.11 ms). + +## Reversal condition + +Reopen this when Unity or macOS stops suppressing a background Editor that receives work over +IPC, or when delays in work that continues after a command has responded (such as Play Mode +tests) are reported as real harm and the hold has to cover more than the command itself. From a0da9d80fdb437434fa5e0fe662765fbdec6658f Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 7 Oct 2026 07:13:32 +0900 Subject: [PATCH 6/6] Pin that the router holds the activity until an asynchronous tool completes The old tool test completed synchronously, so the whole call finished before ExecuteAsync returned, and a router that released the hold before awaiting the tool still passed it. The test now uses a tool whose task stays pending, checks the activity is still held while it is pending, and completes it in finally so a failed assertion leaves nothing behind. --- ...nityCliLoopExecutionRouterActivityTests.cs | 48 ++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs b/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs index f5e893141d..4992e101d3 100644 --- a/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs @@ -18,23 +18,41 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor public sealed class UnityCliLoopExecutionRouterActivityTests { /// - /// Verifies a tool runs while one activity is held, and the activity ends once the tool returns. + /// Verifies the activity stays held while a tool's task is still pending, and ends only once the + /// tool completes, so an asynchronous command is covered for its whole length. /// [Test] - public async Task ExecuteAsync_Tool_HoldsAnActivityWhileTheToolRuns_AndReleasesItAfterwards() + public async Task ExecuteAsync_Tool_HoldsTheActivityWhileTheToolIsPending_AndReleasesItWhenItCompletes() { RecordingProcessActivityApi api = new RecordingProcessActivityApi(); - ActivityObservingTool tool = new ActivityObservingTool(api); + PendingTool tool = new PendingTool(api); UnityCliLoopExecutionRouter router = CreateRouter(api, tool); - UnityCliLoopToolResponse response = await router.ExecuteAsync( - ActivityObservingTool.Name, + Task run = router.ExecuteAsync( + PendingTool.Name, new JObject(), CancellationToken.None); + try + { + // Why these are read before the tool completes: a router that released the hold before + // awaiting the tool would already show the activity as ended here. A tool that completes + // synchronously cannot tell the two apart, because the whole call finishes before it returns. + Assert.That(run.IsCompleted, Is.False); + Assert.That(tool.LiveCountWhileRunning, Is.EqualTo(1)); + Assert.That(api.LiveCount, Is.EqualTo(1)); + Assert.That(api.EndedTokens, Is.Empty); + } + finally + { + // Why in finally: a failed assertion must not leave the tool's task pending. + tool.Complete(new ActivityTestResponse()); + } + + UnityCliLoopToolResponse response = await run; Assert.That(response, Is.InstanceOf()); - Assert.That(tool.LiveCountWhileRunning, Is.EqualTo(1)); Assert.That(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.EndedTokens.Count, Is.EqualTo(1)); Assert.That(api.LiveCount, Is.EqualTo(0)); } @@ -146,15 +164,18 @@ private static UnityCliLoopExecutionRouter CreateRouter( } /// - /// Tool that records how many activities were live while it ran. + /// Tool whose task stays pending until the test completes it, and that records how many + /// activities were live when it started. /// - private sealed class ActivityObservingTool : IUnityCliLoopTool + private sealed class PendingTool : IUnityCliLoopTool { - public const string Name = "activity-observing-test"; + public const string Name = "activity-pending-test"; private readonly RecordingProcessActivityApi _api; + private readonly TaskCompletionSource _completion = + new TaskCompletionSource(); - public ActivityObservingTool(RecordingProcessActivityApi api) + public PendingTool(RecordingProcessActivityApi api) { _api = api; } @@ -168,7 +189,12 @@ public ActivityObservingTool(RecordingProcessActivityApi api) public Task ExecuteAsync(JToken paramsToken, CancellationToken ct) { LiveCountWhileRunning = _api.LiveCount; - return Task.FromResult(new ActivityTestResponse()); + return _completion.Task; + } + + public void Complete(UnityCliLoopToolResponse response) + { + _completion.TrySetResult(response); } }