diff --git a/Assets/Tests/Editor/EditorExecutionActivityTests.cs b/Assets/Tests/Editor/EditorExecutionActivityTests.cs new file mode 100644 index 0000000000..5a11e3a027 --- /dev/null +++ b/Assets/Tests/Editor/EditorExecutionActivityTests.cs @@ -0,0 +1,223 @@ +using System; +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"; + + /// + /// 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(RecordingProcessActivityApi.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)); + } + } +} 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/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/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/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..4992e101d3 --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopExecutionRouterActivityTests.cs @@ -0,0 +1,226 @@ +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 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_HoldsTheActivityWhileTheToolIsPending_AndReleasesItWhenItCompletes() + { + RecordingProcessActivityApi api = new RecordingProcessActivityApi(); + PendingTool tool = new PendingTool(api); + UnityCliLoopExecutionRouter router = CreateRouter(api, tool); + + 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(api.BeginCount, Is.EqualTo(1)); + Assert.That(api.EndedTokens.Count, 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 whose task stays pending until the test completes it, and that records how many + /// activities were live when it started. + /// + private sealed class PendingTool : IUnityCliLoopTool + { + public const string Name = "activity-pending-test"; + + private readonly RecordingProcessActivityApi _api; + private readonly TaskCompletionSource _completion = + new TaskCompletionSource(); + + public PendingTool(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 _completion.Task; + } + + public void Complete(UnityCliLoopToolResponse response) + { + _completion.TrySetResult(response); + } + } + + /// + /// 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/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..7d844f0909 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ExecutionActivity/EditorExecutionActivity.cs @@ -0,0 +1,174 @@ +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 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 = UnityEngine.Application.platform == UnityEngine.RuntimePlatform.OSXEditor + ? new MacProcessActivityApi() + : 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. 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) + { + // 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. 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; + } + + _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: 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: 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, 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.