From 65afbdc3f592ba988bda9f5d8a2bde277498a949 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Dec 2025 22:01:19 +0000 Subject: [PATCH 01/13] Initial plan From b9d78249c9ca11a8960709f1a5c42d3eb7d57b41 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Dec 2025 22:11:30 +0000 Subject: [PATCH 02/13] Implement graceful shutdown for AppiumLocalService on Windows Co-authored-by: Dor-bl <59066376+Dor-bl@users.noreply.github.com> --- .../Appium/Service/AppiumLocalService.cs | 93 ++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index 2ad47a636..898e8e80e 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -22,6 +22,8 @@ using System.Net; using System.Net.Http; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Threading; using System.Threading.Tasks; namespace OpenQA.Selenium.Appium.Service @@ -31,6 +33,24 @@ namespace OpenQA.Selenium.Appium.Service /// public class AppiumLocalService : ICommandServer { + // P/Invoke declarations for Windows graceful shutdown + private const int CTRL_C_EVENT = 0; + private const int CTRL_BREAK_EVENT = 1; + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool AttachConsole(uint dwProcessId); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool FreeConsole(); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool SetConsoleCtrlHandler(ConsoleCtrlDelegate HandlerRoutine, bool Add); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool GenerateConsoleCtrlEvent(uint dwCtrlEvent, uint dwProcessGroupId); + + private delegate bool ConsoleCtrlDelegate(uint CtrlType); + private readonly FileInfo NodeJS; private readonly string Arguments; private readonly IPAddress IP; @@ -144,6 +164,72 @@ private async Task StartAsync() } } + /// + /// Attempts to gracefully shutdown the process on Windows by sending CTRL+C signal. + /// + /// The process to shutdown + /// Timeout in milliseconds to wait for graceful shutdown + /// True if process exited gracefully, false otherwise + private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) + { + if (process == null || process.HasExited) + { + return true; + } + + // Only attempt graceful shutdown on Windows + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return false; + } + + try + { + // Disable Ctrl-C handling for our own process + SetConsoleCtrlHandler(null, true); + + // Attach to the target process console + if (!AttachConsole((uint)process.Id)) + { + return false; + } + + // Send CTRL+C event + if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0)) + { + FreeConsole(); + return false; + } + + // Detach from the process console + FreeConsole(); + + // Re-enable Ctrl-C handling for our own process + SetConsoleCtrlHandler(null, false); + + // Wait for the process to exit gracefully + if (process.WaitForExit(timeoutMs)) + { + return true; + } + } + catch + { + // If anything goes wrong, return false to fallback to Kill() + try + { + FreeConsole(); + SetConsoleCtrlHandler(null, false); + } + catch + { + // Ignore cleanup errors + } + } + + return false; + } + private void DestroyProcess() { if (Service == null) @@ -153,7 +239,12 @@ private void DestroyProcess() try { - Service.Kill(); + // First, attempt graceful shutdown on Windows + if (!TryGracefulShutdownOnWindows(Service)) + { + // If graceful shutdown fails or is not supported, use Kill + Service.Kill(); + } } catch { From d2f3655c2e00437c7af5c75483239c2124f525f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Dec 2025 22:27:45 +0000 Subject: [PATCH 03/13] Address code review feedback: improve error handling and comments Co-authored-by: Dor-bl <59066376+Dor-bl@users.noreply.github.com> --- src/Appium.Net/Appium/Service/AppiumLocalService.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index 898e8e80e..89e7c454d 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -16,6 +16,7 @@ using OpenQA.Selenium.Remote; using System; using System.Collections.Generic; +using System.ComponentModel; using System.Diagnostics; using System.IO; using System.Linq; @@ -35,7 +36,6 @@ public class AppiumLocalService : ICommandServer { // P/Invoke declarations for Windows graceful shutdown private const int CTRL_C_EVENT = 0; - private const int CTRL_BREAK_EVENT = 1; [DllImport("kernel32.dll", SetLastError = true)] private static extern bool AttachConsole(uint dwProcessId); @@ -185,7 +185,8 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) try { - // Disable Ctrl-C handling for our own process + // Disable Ctrl-C handling for our own process to prevent it from being terminated + // when we send CTRL+C to the target process SetConsoleCtrlHandler(null, true); // Attach to the target process console @@ -213,9 +214,9 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) return true; } } - catch + catch (Exception ex) when (ex is Win32Exception || ex is InvalidOperationException) { - // If anything goes wrong, return false to fallback to Kill() + // If Windows API calls fail or process has already exited, return false to fallback to Kill() try { FreeConsole(); @@ -223,7 +224,7 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) } catch { - // Ignore cleanup errors + // Ignore cleanup errors - we're already in error handling } } From 77577ebb983067af2e24872710906cca8e4eb542 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 14 Dec 2025 22:32:04 +0000 Subject: [PATCH 04/13] Further improve exception handling with separate catch blocks Co-authored-by: Dor-bl <59066376+Dor-bl@users.noreply.github.com> --- .../Appium/Service/AppiumLocalService.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index 89e7c454d..3ccbd84f8 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -214,15 +214,28 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) return true; } } - catch (Exception ex) when (ex is Win32Exception || ex is InvalidOperationException) + catch (Win32Exception) { - // If Windows API calls fail or process has already exited, return false to fallback to Kill() + // Windows API call failed, return false to fallback to Kill() try { FreeConsole(); SetConsoleCtrlHandler(null, false); } - catch + catch (Win32Exception) + { + // Ignore cleanup errors - we're already in error handling + } + } + catch (InvalidOperationException) + { + // Process has already exited, return false to fallback to Kill() + try + { + FreeConsole(); + SetConsoleCtrlHandler(null, false); + } + catch (Win32Exception) { // Ignore cleanup errors - we're already in error handling } From 54d3cb5a175b673b1a9df8fbf674b52bf7167e1f Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sat, 7 Feb 2026 23:02:50 +0100 Subject: [PATCH 05/13] Improve process shutdown handling in AppiumLocalService Updated `TryGracefulShutdownOnWindows` to enhance null checks and exception handling for disposed processes. Simplified CTRL+C event handling and removed unnecessary comments for clarity. Added a new test class in `AppiumLocalServiceTests` to validate the behavior of `TryGracefulShutdownOnWindows`. The tests cover various scenarios, including null processes, exited processes, non-Windows platforms, and real Appium processes, using reflection to access private members. --- .../Appium/Service/AppiumLocalService.cs | 48 +++--- .../ServerTests/AppiumLocalServiceTests.cs | 146 ++++++++++++++++++ 2 files changed, 174 insertions(+), 20 deletions(-) create mode 100644 test/integration/ServerTests/AppiumLocalServiceTests.cs diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index 3ccbd84f8..accfcfc54 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -24,7 +24,6 @@ using System.Net.Http; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Threading; using System.Threading.Tasks; namespace OpenQA.Selenium.Appium.Service @@ -172,8 +171,33 @@ private async Task StartAsync() /// True if process exited gracefully, false otherwise private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) { - if (process == null || process.HasExited) + try { + if (process == null) + { + return true; + } + + // Safely check HasExited, handling disposed process + bool hasExited; + try + { + hasExited = process.HasExited; + } + catch (InvalidOperationException) + { + // Process is disposed, treat as exited + return true; + } + + if (hasExited) + { + return true; + } + } + catch (InvalidOperationException) + { + // Process is disposed, treat as exited return true; } @@ -185,30 +209,22 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) try { - // Disable Ctrl-C handling for our own process to prevent it from being terminated - // when we send CTRL+C to the target process SetConsoleCtrlHandler(null, true); - // Attach to the target process console if (!AttachConsole((uint)process.Id)) { return false; } - // Send CTRL+C event if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0)) { FreeConsole(); return false; } - // Detach from the process console FreeConsole(); - - // Re-enable Ctrl-C handling for our own process SetConsoleCtrlHandler(null, false); - // Wait for the process to exit gracefully if (process.WaitForExit(timeoutMs)) { return true; @@ -216,29 +232,21 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) } catch (Win32Exception) { - // Windows API call failed, return false to fallback to Kill() try { FreeConsole(); SetConsoleCtrlHandler(null, false); } - catch (Win32Exception) - { - // Ignore cleanup errors - we're already in error handling - } + catch (Win32Exception) { } } catch (InvalidOperationException) { - // Process has already exited, return false to fallback to Kill() try { FreeConsole(); SetConsoleCtrlHandler(null, false); } - catch (Win32Exception) - { - // Ignore cleanup errors - we're already in error handling - } + catch (Win32Exception) { } } return false; diff --git a/test/integration/ServerTests/AppiumLocalServiceTests.cs b/test/integration/ServerTests/AppiumLocalServiceTests.cs new file mode 100644 index 000000000..b15d442db --- /dev/null +++ b/test/integration/ServerTests/AppiumLocalServiceTests.cs @@ -0,0 +1,146 @@ +using NUnit.Framework; +using System; +using System.Diagnostics; +using System.IO; +using System.Reflection; +using System.Runtime.InteropServices; +using OpenQA.Selenium.Appium.Service; +using OpenQA.Selenium.Appium.Service.Options; + +namespace Appium.Net.Unit.Tests.Appium.Service +{ + [TestFixture] + public class AppiumLocalServiceTests + { + private AppiumLocalService appiumServer; + + [OneTimeSetUp] + public void GlobalSetup() + { + // OptionCollector can be customized as needed + var optionCollector = new OptionCollector(); + var appiumPath = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? @"C:\Users\Dor-B\AppData\Roaming\npm\node_modules\appium\build\lib\main.js" + : "/usr/local/lib/node_modules/appium/build/lib/main.js"; + appiumServer = new AppiumServiceBuilder() + .WithAppiumJS(new FileInfo(appiumPath)) + .WithIPAddress("127.0.0.1") + .UsingAnyFreePort() + .WithArguments(optionCollector) + .Build(); + appiumServer.Start(); + + // Check that the server is running after startup + Assert.That(appiumServer.IsRunning, Is.True, "Appium server should be running after Start()"); + } + + [OneTimeTearDown] + public void GlobalTeardown() + { + appiumServer.Dispose(); + } + + private AppiumLocalService CreateService() + { + // Use dummy values for constructor + return (AppiumLocalService)Activator.CreateInstance( + typeof(AppiumLocalService), + BindingFlags.Instance | BindingFlags.NonPublic, + null, + new object[] + { + new System.IO.FileInfo("node"), + "", + System.Net.IPAddress.Loopback, + 4723, + TimeSpan.FromSeconds(5), + null + }, + null + ); + } + + private MethodInfo GetTryGracefulShutdownOnWindows() + { + return typeof(AppiumLocalService).GetMethod( + "TryGracefulShutdownOnWindows", + BindingFlags.Instance | BindingFlags.NonPublic + ); + } + + [Test] + public void TryGracefulShutdownOnWindows_ReturnsTrue_WhenProcessIsNull() + { + var service = CreateService(); + var method = GetTryGracefulShutdownOnWindows(); + var result = (bool)method.Invoke(service, new object[] { null, 5000 }); + Assert.That(result, Is.True); + } + + [Test] + public void TryGracefulShutdownOnWindows_ReturnsTrue_WhenProcessHasExited() + { + var service = CreateService(); + var method = GetTryGracefulShutdownOnWindows(); + + var proc = new Process(); + // Simulate HasExited = true by disposing process (not perfect, but for test) + proc.Dispose(); + + // Use try-catch because accessing HasExited on disposed process throws + bool result; + try + { + result = (bool)method.Invoke(service, new object[] { proc, 5000 }); + } + catch (TargetInvocationException ex) when (ex.InnerException is InvalidOperationException) + { + // If exception, treat as HasExited = true + result = true; + } + Assert.That(result, Is.True); + } + + [Test] + public void TryGracefulShutdownOnWindows_ReturnsFalse_WhenNotWindows() + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Ignore("Test only valid on non-Windows platforms."); + } + var service = CreateService(); + var method = GetTryGracefulShutdownOnWindows(); + + // Use a running process (self) + using (var proc = Process.GetCurrentProcess()) + { + var result = (bool)method.Invoke(service, new object[] { proc, 5000 }); + Assert.That(result, Is.False); + } + } + + [Test] + public void TryGracefulShutdownOnWindows_RealProcess_DoesNotThrow() + { + var method = typeof(AppiumLocalService) + .GetMethod("TryGracefulShutdownOnWindows", BindingFlags.Instance | BindingFlags.NonPublic); + + // This test will only run on Windows + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Ignore("Test only valid on Windows platforms."); + } + + // Use reflection to call the private method on the real Appium process + var result = (bool)method.Invoke(appiumServer, new object[] { GetAppiumProcess(appiumServer), 5000 }); + Assert.That(result, Is.True.Or.False, "Method should not throw and should return a bool."); + } + + private Process GetAppiumProcess(AppiumLocalService service) + { + // Use reflection to access the private 'Service' field + var field = typeof(AppiumLocalService).GetField("Service", BindingFlags.Instance | BindingFlags.NonPublic); + return (Process)field.GetValue(service); + } + } +} \ No newline at end of file From fed3efb9dde6dda2d064b5b67ad959aa7bfb74be Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sat, 7 Feb 2026 23:14:59 +0100 Subject: [PATCH 06/13] Refactor process handling in AppiumLocalServiceTests Removed the `using` statement for the current process in `AppiumLocalServiceTests.cs`, allowing it to be referenced without disposal. This simplifies the code while maintaining the functionality to test graceful shutdown behavior on Windows. --- .../integration/ServerTests/AppiumLocalServiceTests.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/integration/ServerTests/AppiumLocalServiceTests.cs b/test/integration/ServerTests/AppiumLocalServiceTests.cs index b15d442db..d9dbcd229 100644 --- a/test/integration/ServerTests/AppiumLocalServiceTests.cs +++ b/test/integration/ServerTests/AppiumLocalServiceTests.cs @@ -111,12 +111,10 @@ public void TryGracefulShutdownOnWindows_ReturnsFalse_WhenNotWindows() var service = CreateService(); var method = GetTryGracefulShutdownOnWindows(); - // Use a running process (self) - using (var proc = Process.GetCurrentProcess()) - { - var result = (bool)method.Invoke(service, new object[] { proc, 5000 }); - Assert.That(result, Is.False); - } + // Use a running process (self) without disposing it + var proc = Process.GetCurrentProcess(); + var result = (bool)method.Invoke(service, new object[] { proc, 5000 }); + Assert.That(result, Is.False); } [Test] From d7c8cc9f14e0199408eeccee96e5303864ef9a01 Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sat, 7 Feb 2026 23:20:03 +0100 Subject: [PATCH 07/13] Refactor console attachment handling in AppiumLocalService Added a boolean variable to track console attachment status. Simplified error handling by removing redundant try-catch blocks and ensuring proper cleanup in the finally block. --- .../Appium/Service/AppiumLocalService.cs | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index accfcfc54..3a8372527 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -207,46 +207,34 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) return false; } + bool attached = false; try { SetConsoleCtrlHandler(null, true); - if (!AttachConsole((uint)process.Id)) + attached = AttachConsole((uint)process.Id); + if (!attached) { return false; } if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0)) { - FreeConsole(); return false; } - FreeConsole(); - SetConsoleCtrlHandler(null, false); - if (process.WaitForExit(timeoutMs)) { return true; } } - catch (Win32Exception) - { - try - { - FreeConsole(); - SetConsoleCtrlHandler(null, false); - } - catch (Win32Exception) { } - } - catch (InvalidOperationException) + finally { - try + if (attached) { FreeConsole(); - SetConsoleCtrlHandler(null, false); } - catch (Win32Exception) { } + SetConsoleCtrlHandler(null, false); } return false; From 3d1c6f6e7317a324a0882797c67ff01b38c5987d Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sat, 7 Feb 2026 23:35:13 +0100 Subject: [PATCH 08/13] Refactor AppiumLocalServiceTests namespace and setup Changed the namespace of `AppiumLocalServiceTests` to `Appium.Net.Integration.Tests.ServerTests`. Updated the `GlobalSetup` method to dynamically determine the Appium path using an environment variable or by executing `npm root -g`, removing hardcoded paths. If the Appium path is invalid, the test is skipped with an appropriate message. --- .../ServerTests/AppiumLocalServiceTests.cs | 51 +++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/test/integration/ServerTests/AppiumLocalServiceTests.cs b/test/integration/ServerTests/AppiumLocalServiceTests.cs index d9dbcd229..f16f9b070 100644 --- a/test/integration/ServerTests/AppiumLocalServiceTests.cs +++ b/test/integration/ServerTests/AppiumLocalServiceTests.cs @@ -7,7 +7,7 @@ using OpenQA.Selenium.Appium.Service; using OpenQA.Selenium.Appium.Service.Options; -namespace Appium.Net.Unit.Tests.Appium.Service +namespace Appium.Net.Integration.Tests.ServerTests { [TestFixture] public class AppiumLocalServiceTests @@ -19,9 +19,52 @@ public void GlobalSetup() { // OptionCollector can be customized as needed var optionCollector = new OptionCollector(); - var appiumPath = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? @"C:\Users\Dor-B\AppData\Roaming\npm\node_modules\appium\build\lib\main.js" - : "/usr/local/lib/node_modules/appium/build/lib/main.js"; + + // Try to get Appium path from environment variable or npm root -g + string appiumPath = Environment.GetEnvironmentVariable(AppiumServiceConstants.AppiumBinaryPath); + + if (string.IsNullOrEmpty(appiumPath)) + { + try + { + bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + + var startInfo = new ProcessStartInfo + { + // On Windows, using 'cmd /c' is often more reliable for resolving PATHs + FileName = isWindows ? "cmd.exe" : "npm", + Arguments = isWindows ? "/c npm root -g" : "root -g", + RedirectStandardOutput = true, + RedirectStandardError = true, // Capture errors too! + UseShellExecute = false, + CreateNoWindow = true + }; + + using var process = Process.Start(startInfo); + string output = process.StandardOutput.ReadToEnd()?.Trim(); + string error = process.StandardError.ReadToEnd()?.Trim(); + process.WaitForExit(); + + if (!string.IsNullOrEmpty(output)) + { + appiumPath = Path.Combine(output, "appium", "build", "lib", "main.js"); + } + else if (!string.IsNullOrEmpty(error)) + { + Console.WriteLine($"NPM Error: {error}"); + } + } + catch (Exception ex) + { + Console.WriteLine($"Process failed: {ex.Message}"); + } + } + + if (string.IsNullOrEmpty(appiumPath) || !File.Exists(appiumPath)) + { + Assert.Ignore("Appium is not installed or not found on this machine. Skipping AppiumLocalServiceTests."); + } + appiumServer = new AppiumServiceBuilder() .WithAppiumJS(new FileInfo(appiumPath)) .WithIPAddress("127.0.0.1") From bd92a11e80f91aebd339e71e7d38404187d7963d Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sat, 7 Feb 2026 23:40:54 +0100 Subject: [PATCH 09/13] Refactor process shutdown logic for Windows Removed P/Invoke declarations and related methods for graceful shutdown. Simplified `TryGracefulShutdownOnWindows` to check process exit status without console attachment. Adjusted exception handling to fallback to process termination, streamlining the shutdown process. --- .../Appium/Service/AppiumLocalService.cs | 91 +++---------------- 1 file changed, 14 insertions(+), 77 deletions(-) diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index 3a8372527..59b18ccc3 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -16,14 +16,12 @@ using OpenQA.Selenium.Remote; using System; using System.Collections.Generic; -using System.ComponentModel; using System.Diagnostics; using System.IO; using System.Linq; using System.Net; using System.Net.Http; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Threading.Tasks; namespace OpenQA.Selenium.Appium.Service @@ -33,23 +31,6 @@ namespace OpenQA.Selenium.Appium.Service /// public class AppiumLocalService : ICommandServer { - // P/Invoke declarations for Windows graceful shutdown - private const int CTRL_C_EVENT = 0; - - [DllImport("kernel32.dll", SetLastError = true)] - private static extern bool AttachConsole(uint dwProcessId); - - [DllImport("kernel32.dll", SetLastError = true)] - private static extern bool FreeConsole(); - - [DllImport("kernel32.dll", SetLastError = true)] - private static extern bool SetConsoleCtrlHandler(ConsoleCtrlDelegate HandlerRoutine, bool Add); - - [DllImport("kernel32.dll", SetLastError = true)] - private static extern bool GenerateConsoleCtrlEvent(uint dwCtrlEvent, uint dwProcessGroupId); - - private delegate bool ConsoleCtrlDelegate(uint CtrlType); - private readonly FileInfo NodeJS; private readonly string Arguments; private readonly IPAddress IP; @@ -163,37 +144,16 @@ private async Task StartAsync() } } - /// - /// Attempts to gracefully shutdown the process on Windows by sending CTRL+C signal. - /// - /// The process to shutdown - /// Timeout in milliseconds to wait for graceful shutdown - /// True if process exited gracefully, false otherwise private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) { + if (process == null) + return true; + + // Safely check HasExited, handling disposed process + bool hasExited; try { - if (process == null) - { - return true; - } - - // Safely check HasExited, handling disposed process - bool hasExited; - try - { - hasExited = process.HasExited; - } - catch (InvalidOperationException) - { - // Process is disposed, treat as exited - return true; - } - - if (hasExited) - { - return true; - } + hasExited = process.HasExited; } catch (InvalidOperationException) { @@ -201,40 +161,19 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) return true; } - // Only attempt graceful shutdown on Windows - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - return false; - } + if (hasExited) + return true; - bool attached = false; + // Attempt graceful shutdown using managed code only try { - SetConsoleCtrlHandler(null, true); - - attached = AttachConsole((uint)process.Id); - if (!attached) - { - return false; - } - - if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0)) - { - return false; - } - + process.CloseMainWindow(); if (process.WaitForExit(timeoutMs)) - { return true; - } } - finally + catch { - if (attached) - { - FreeConsole(); - } - SetConsoleCtrlHandler(null, false); + // Ignore exceptions, fallback to Kill } return false; @@ -243,16 +182,14 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) private void DestroyProcess() { if (Service == null) - { return; - } try { - // First, attempt graceful shutdown on Windows + // First, attempt graceful shutdown using managed code if (!TryGracefulShutdownOnWindows(Service)) { - // If graceful shutdown fails or is not supported, use Kill + // If graceful shutdown fails, use Kill Service.Kill(); } } From ec9a8f2c638d32a82dc33a9a32d496fadad78c12 Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sat, 7 Feb 2026 23:46:06 +0100 Subject: [PATCH 10/13] Add method existence checks in shutdown tests This commit introduces assertions to verify the presence of the `TryGracefulShutdownOnWindows` method in multiple test cases. This change helps identify potential issues with method signatures or names. Additionally, unnecessary comments and code related to simulating process states have been removed or modified to enhance the clarity and focus of the tests. --- .../integration/ServerTests/AppiumLocalServiceTests.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/integration/ServerTests/AppiumLocalServiceTests.cs b/test/integration/ServerTests/AppiumLocalServiceTests.cs index f16f9b070..d6fd2015c 100644 --- a/test/integration/ServerTests/AppiumLocalServiceTests.cs +++ b/test/integration/ServerTests/AppiumLocalServiceTests.cs @@ -116,6 +116,7 @@ public void TryGracefulShutdownOnWindows_ReturnsTrue_WhenProcessIsNull() { var service = CreateService(); var method = GetTryGracefulShutdownOnWindows(); + Assert.That(method, Is.Not.Null, "TryGracefulShutdownOnWindows method not found. Check for signature or name changes."); var result = (bool)method.Invoke(service, new object[] { null, 5000 }); Assert.That(result, Is.True); } @@ -125,12 +126,11 @@ public void TryGracefulShutdownOnWindows_ReturnsTrue_WhenProcessHasExited() { var service = CreateService(); var method = GetTryGracefulShutdownOnWindows(); + Assert.That(method, Is.Not.Null, "TryGracefulShutdownOnWindows method not found. Check for signature or name changes."); var proc = new Process(); - // Simulate HasExited = true by disposing process (not perfect, but for test) proc.Dispose(); - // Use try-catch because accessing HasExited on disposed process throws bool result; try { @@ -138,7 +138,6 @@ public void TryGracefulShutdownOnWindows_ReturnsTrue_WhenProcessHasExited() } catch (TargetInvocationException ex) when (ex.InnerException is InvalidOperationException) { - // If exception, treat as HasExited = true result = true; } Assert.That(result, Is.True); @@ -153,8 +152,8 @@ public void TryGracefulShutdownOnWindows_ReturnsFalse_WhenNotWindows() } var service = CreateService(); var method = GetTryGracefulShutdownOnWindows(); + Assert.That(method, Is.Not.Null, "TryGracefulShutdownOnWindows method not found. Check for signature or name changes."); - // Use a running process (self) without disposing it var proc = Process.GetCurrentProcess(); var result = (bool)method.Invoke(service, new object[] { proc, 5000 }); Assert.That(result, Is.False); @@ -165,14 +164,13 @@ public void TryGracefulShutdownOnWindows_RealProcess_DoesNotThrow() { var method = typeof(AppiumLocalService) .GetMethod("TryGracefulShutdownOnWindows", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(method, Is.Not.Null, "TryGracefulShutdownOnWindows method not found. Check for signature or name changes."); - // This test will only run on Windows if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { Assert.Ignore("Test only valid on Windows platforms."); } - // Use reflection to call the private method on the real Appium process var result = (bool)method.Invoke(appiumServer, new object[] { GetAppiumProcess(appiumServer), 5000 }); Assert.That(result, Is.True.Or.False, "Method should not throw and should return a bool."); } From d0bf7812238398783574396428b2e42800430c11 Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sat, 7 Feb 2026 23:47:04 +0100 Subject: [PATCH 11/13] Use null-conditional operator for appiumServer.Dispose Modified the GlobalTeardown method to use the null-conditional operator (`?.`) when calling `Dispose` on `appiumServer`. This change prevents potential `NullReferenceException` by ensuring that `Dispose` is only called if `appiumServer` is not null. --- test/integration/ServerTests/AppiumLocalServiceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/ServerTests/AppiumLocalServiceTests.cs b/test/integration/ServerTests/AppiumLocalServiceTests.cs index d6fd2015c..ca4067138 100644 --- a/test/integration/ServerTests/AppiumLocalServiceTests.cs +++ b/test/integration/ServerTests/AppiumLocalServiceTests.cs @@ -80,7 +80,7 @@ public void GlobalSetup() [OneTimeTearDown] public void GlobalTeardown() { - appiumServer.Dispose(); + appiumServer?.Dispose(); } private AppiumLocalService CreateService() From e398edee56f61d1349b2b6f6010bfe68719facd9 Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sun, 8 Feb 2026 20:45:11 +0100 Subject: [PATCH 12/13] Enhance service shutdown logic for OS compatibility Added System.Runtime.InteropServices for platform checks. Updated shutdown logic to handle Windows-specific graceful shutdown and included exception handling for robustness. --- .../Appium/Service/AppiumLocalService.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index 59b18ccc3..f2425fb11 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -22,6 +22,7 @@ using System.Net; using System.Net.Http; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Threading.Tasks; namespace OpenQA.Selenium.Appium.Service @@ -186,15 +187,23 @@ private void DestroyProcess() try { - // First, attempt graceful shutdown using managed code - if (!TryGracefulShutdownOnWindows(Service)) + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - // If graceful shutdown fails, use Kill + // Attempt graceful shutdown on Windows + if (!TryGracefulShutdownOnWindows(Service)) + { + Service.Kill(); + } + } + else + { + // On non-Windows, just kill the process Service.Kill(); } } catch { + // Optionally log or handle exceptions } finally { From 8cb70346b2a66833488042b12eb9e441699e803a Mon Sep 17 00:00:00 2001 From: Dor Blayzer Date: Sun, 8 Feb 2026 20:50:25 +0100 Subject: [PATCH 13/13] Add shutdown timeout handling in AppiumLocalService Introduced a new method `GetShutdownTimeoutWithBuffer` to calculate the shutdown timeout for the Appium service, incorporating a default timeout of 5000 ms and an additional buffer of 1000 ms. This method checks for the `--shutdown-timeout` argument to allow customization. The timeout value is utilized in the `DestroyProcess` method for graceful shutdown on Windows. --- .../Appium/Service/AppiumLocalService.cs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs index f2425fb11..977f6fb1c 100644 --- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs +++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs @@ -180,6 +180,25 @@ private bool TryGracefulShutdownOnWindows(Process process, int timeoutMs = 5000) return false; } + private int GetShutdownTimeoutWithBuffer() + { + // Default Appium shutdown timeout in ms + int shutdownTimeout = 5000; + const int bufferMs = 1000; + + if (ArgsList == null) + GenerateArgsList(); + + int idx = ArgsList.IndexOf("--shutdown-timeout"); + if (idx >= 0 && idx + 1 < ArgsList.Count) + { + if (int.TryParse(ArgsList[idx + 1], out int parsed)) + shutdownTimeout = parsed; + } + + return shutdownTimeout + bufferMs; + } + private void DestroyProcess() { if (Service == null) @@ -187,10 +206,12 @@ private void DestroyProcess() try { + int shutdownTimeout = GetShutdownTimeoutWithBuffer(); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { // Attempt graceful shutdown on Windows - if (!TryGracefulShutdownOnWindows(Service)) + if (!TryGracefulShutdownOnWindows(Service, shutdownTimeout)) { Service.Kill(); }