fix: Stop local appium process with SIGTERM #1006

Dor-bl · 2026-02-07T22:10:16Z

List of changes

This pull request introduces a new mechanism for gracefully shutting down the Appium server process on Windows, using a CTRL+C signal instead of a forced kill. It also adds comprehensive tests for this shutdown logic to ensure robust behavior across platforms and edge cases.

Supposedly should fix: #625

Graceful shutdown enhancements:

Added Windows-specific P/Invoke declarations and a new method TryGracefulShutdownOnWindows to attempt a graceful shutdown of the Appium process using CTRL+C before resorting to a forced kill. This improves reliability and prevents abrupt process termination. (src/Appium.Net/Appium/Service/AppiumLocalService.cs) [1] [2]
Updated the DestroyProcess method to use the graceful shutdown logic, falling back to Kill() only if necessary. (src/Appium.Net/Appium/Service/AppiumLocalService.cs)

Testing improvements:

Added a new test class AppiumLocalServiceTests with multiple tests to verify the graceful shutdown logic, including edge cases for null processes, disposed processes, platform-specific behavior, and real Appium server shutdown. (test/integration/ServerTests/AppiumLocalServiceTests.cs)

Dependency updates:

Added System.ComponentModel and System.Runtime.InteropServices namespaces to support P/Invoke and platform checks. (src/Appium.Net/Appium/Service/AppiumLocalService.cs)

Types of changes

What types of changes are you proposing/introducing to the .NET client?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change that adds functionality or value)
Breaking change (fix or feature that would cause existing functionality not to work as expected)
Test fix (non-breaking change that improves test stability or correctness)

Documentation

Have you proposed a file change/ PR with Appium to update documentation?

This can be done by navigating to the documentation section on http://appium.io selecting the appropriate command/endpoint and clicking the 'Edit this doc' link to update the C# example

Integration tests

Have you provided integration tests for your changes? (required for Bugfix, New feature, or Test fix)

Details

Please provide more details about changes if necessary. You can provide code samples showing how they work and possible use cases if there are new features. Also, you can create gists with pasted C# code samples or put them here using markdown.
About markdown please read Mastering markdown and Writing on GitHub

Co-authored-by: Dor-bl <59066376+Dor-bl@users.noreply.github.com>

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.

linux-foundation-easycla · 2026-02-07T22:10:21Z

✅ login: Dor-bl / name: Dor Blayzer (3d1c6f6, 54d3cb5, 8cb7034, bd92a11, d0bf781, d7c8cc9, e398ede, ec9a8f2, fed3efb)
❌ - login: @copilot / name: copilot-swe-agent[bot] . The commit (65afbdc, 77577eb, b9d7824, d2f3655) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

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.

Copilot

Pull request overview

This PR updates AppiumLocalService shutdown behavior to attempt a more graceful stop on Windows by sending a console CTRL+C event before falling back to forcibly killing the process, and adds new tests around this shutdown behavior.

Changes:

Added Windows P/Invoke plumbing and a TryGracefulShutdownOnWindows helper that attempts to stop the Appium process via CTRL+C.
Updated DestroyProcess to prefer graceful shutdown on Windows, with fallback to Process.Kill().
Added a new integration test fixture intended to validate the Windows graceful-shutdown logic.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 23 comments.

File	Description
src/Appium.Net/Appium/Service/AppiumLocalService.cs	Adds Windows-specific graceful shutdown logic and integrates it into process teardown.
test/integration/ServerTests/AppiumLocalServiceTests.cs	Introduces tests (via reflection + a real Appium process) for the graceful shutdown helper and edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/integration/ServerTests/AppiumLocalServiceTests.cs

src/Appium.Net/Appium/Service/AppiumLocalService.cs

test/integration/ServerTests/AppiumLocalServiceTests.cs

src/Appium.Net/Appium/Service/AppiumLocalService.cs

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.

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.

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.

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.

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.

src/Appium.Net/Appium/Service/AppiumLocalService.cs

Added System.Runtime.InteropServices for platform checks. Updated shutdown logic to handle Windows-specific graceful shutdown and included exception handling for robustness.

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.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T19:55:10Z

test/integration/ServerTests/AppiumLocalServiceTests.cs

+            // OptionCollector can be customized as needed
+            var optionCollector = new OptionCollector();
+
+            // 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")
+                .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()");


OneTimeSetUp always starts a real Appium server even though only one test (TryGracefulShutdownOnWindows_RealProcess_DoesNotThrow) needs it. This makes the whole suite slower and causes all tests to be skipped if Appium isn’t available. Consider starting the server only in the specific test that requires it (or moving the real-process test into its own fixture).

Suggested change

// OptionCollector can be customized as needed

var optionCollector = new OptionCollector();

// 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")

.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()");

// Intentionally left empty.

// Starting a real Appium server in OneTimeSetUp makes all tests in this fixture

// depend on a local Appium installation and pay the startup cost, even when

// they do not require a real server. Any test that truly needs a running

// Appium server should start (and stop) it explicitly within that test.

Copilot · 2026-02-08T19:55:10Z

test/integration/ServerTests/AppiumLocalServiceTests.cs

+        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
+            );
+        }


CreateService constructs an AppiumLocalService (which creates an HttpClient) but the created instances are never disposed in the tests. Dispose the service instances (e.g., via using/try-finally) to avoid resource leaks across the test run.

Copilot · 2026-02-08T19:55:11Z

test/integration/ServerTests/AppiumLocalServiceTests.cs

+            var proc = new Process();
+            proc.Dispose();
+
+            bool result;
+            try
+            {
+                result = (bool)method.Invoke(service, new object[] { proc, 5000 });
+            }
+            catch (TargetInvocationException ex) when (ex.InnerException is InvalidOperationException)
+            {
+                result = true;
+            }
+            Assert.That(result, Is.True);
+        }


This test disposes a Process and then expects an InvalidOperationException path. In practice, invoking TryGracefulShutdownOnWindows may surface ObjectDisposedException (or a different exception type) depending on runtime implementation, which would make the test flaky. Prefer creating a real short-lived process that exits, or explicitly assert/handle ObjectDisposedException if the intention is to cover disposed processes.

Copilot · 2026-02-08T19:55:11Z

src/Appium.Net/Appium/Service/AppiumLocalService.cs

+            // Attempt graceful shutdown using managed code only
+            try
+            {
+                process.CloseMainWindow();
+                if (process.WaitForExit(timeoutMs))
+                    return true;


TryGracefulShutdownOnWindows uses CloseMainWindow/WaitForExit, but the Appium process is started with CreateNoWindow=true (console Node process), so it typically has no main window handle and CloseMainWindow will be a no-op. This means the method will almost always fall back to Kill() and won't provide the intended graceful shutdown. Consider implementing actual console CTRL+C/CTRL+BREAK delivery (e.g., GenerateConsoleCtrlEvent + process group setup) or another Appium-supported graceful stop mechanism.

Copilot · 2026-02-08T19:55:11Z

src/Appium.Net/Appium/Service/AppiumLocalService.cs

+                hasExited = process.HasExited;
+            }
+            catch (InvalidOperationException)
+            {


The HasExited access only catches InvalidOperationException, but a disposed/closed Process can throw ObjectDisposedException. If that happens, this method will throw instead of treating the process as already stopped (which is what the comments imply). Catch ObjectDisposedException as well (and possibly treat unstarted processes similarly).

Suggested change

{

{

// Process is not running / not started; treat as exited

return true;

}

catch (ObjectDisposedException)

{

Copilot · 2026-02-08T19:55:11Z

src/Appium.Net/Appium/Service/AppiumLocalService.cs

+                if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+                {
+                    // Attempt graceful shutdown on Windows
+                    if (!TryGracefulShutdownOnWindows(Service, shutdownTimeout))
+                    {
+                        Service.Kill();
+                    }
+                }
+                else
+                {
+                    // On non-Windows, just kill the process
+                    Service.Kill();
+                }


The PR description/title mention SIGTERM/CTRL+C-based shutdown, but DestroyProcess still uses Kill() on non-Windows (SIGKILL on Unix) and the Windows path uses CloseMainWindow rather than CTRL+C. Either update the implementation to match the intended signal-based shutdown behavior or adjust the PR description/title to reflect what is actually implemented.

Copilot · 2026-02-08T19:55:12Z

test/integration/ServerTests/AppiumLocalServiceTests.cs

+                    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}");


GlobalSetup runs npm root -g and then calls process.WaitForExit() with no timeout. If npm hangs (or prompts), this will hang the entire integration test run. Use a bounded wait (and/or the existing helper that already has a timeout) and fail/ignore cleanly on timeout.

Suggested change

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}");

if (process == null)

{

Console.WriteLine("Failed to start npm process to locate global Appium installation.");

}

else

{

const int npmTimeoutMilliseconds = 15000;

if (!process.WaitForExit(npmTimeoutMilliseconds))

{

try

{

process.Kill();

}

catch (Exception killEx)

{

Console.WriteLine($"Failed to kill npm process after timeout: {killEx.Message}");

}

Assert.Ignore(

$"Timed out waiting for 'npm root -g' to complete after {npmTimeoutMilliseconds} ms. " +

"Skipping AppiumLocalServiceTests to avoid hanging the test run.");

}

string output = process.StandardOutput.ReadToEnd()?.Trim();

string error = process.StandardError.ReadToEnd()?.Trim();

if (!string.IsNullOrEmpty(output))

{

appiumPath = Path.Combine(output, "appium", "build", "lib", "main.js");

}

else if (!string.IsNullOrEmpty(error))

{

Console.WriteLine($"NPM Error: {error}");

}

Copilot · 2026-02-08T19:55:12Z

test/integration/ServerTests/AppiumLocalServiceTests.cs

+            // OptionCollector can be customized as needed
+            var optionCollector = new OptionCollector();
+
+            // 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}");
+                    }
+                }


This setup duplicates logic that already exists in test/integration/helpers (Paths + Npm.GetNpmPrefixPath). Reusing the shared helper would reduce duplication and keep Appium path resolution consistent across integration tests.

Copilot · 2026-02-08T19:55:12Z

src/Appium.Net/Appium/Service/AppiumLocalService.cs

+            if (idx >= 0 && idx + 1 < ArgsList.Count)
+            {
+                if (int.TryParse(ArgsList[idx + 1], out int parsed))
+                    shutdownTimeout = parsed;


These 'if' statements can be combined.

Suggested change

if (idx >= 0 && idx + 1 < ArgsList.Count)

{

if (int.TryParse(ArgsList[idx + 1], out int parsed))

shutdownTimeout = parsed;

if (idx >= 0 && idx + 1 < ArgsList.Count &&

int.TryParse(ArgsList[idx + 1], out int parsed))

{

shutdownTimeout = parsed;

This change looks reasonable to me

KazuCocoa

lgtm entirely in my best knowledge

KazuCocoa · 2026-02-09T02:39:08Z

src/Appium.Net/Appium/Service/AppiumLocalService.cs

+            if (idx >= 0 && idx + 1 < ArgsList.Count)
+            {
+                if (int.TryParse(ArgsList[idx + 1], out int parsed))
+                    shutdownTimeout = parsed;


This change looks reasonable to me

Copilot AI and others added 5 commits December 14, 2025 22:01

Initial plan

65afbdc

Implement graceful shutdown for AppiumLocalService on Windows

b9d7824

Co-authored-by: Dor-bl <59066376+Dor-bl@users.noreply.github.com>

Address code review feedback: improve error handling and comments

d2f3655

Co-authored-by: Dor-bl <59066376+Dor-bl@users.noreply.github.com>

Further improve exception handling with separate catch blocks

77577eb

Co-authored-by: Dor-bl <59066376+Dor-bl@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 7, 2026 22:10

github-actions bot added the BugFix label Feb 7, 2026

Copilot started reviewing on behalf of Dor-bl February 7, 2026 22:10 View session

Dor-bl requested review from KazuCocoa and mykola-mokhnach February 7, 2026 22:11

Copilot AI reviewed Feb 7, 2026

View reviewed changes

Dor-bl added 5 commits February 7, 2026 23:20

Refactor console attachment handling in AppiumLocalService

d7c8cc9

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.

KazuCocoa reviewed Feb 8, 2026

View reviewed changes

src/Appium.Net/Appium/Service/AppiumLocalService.cs Outdated Show resolved Hide resolved

mykola-mokhnach reviewed Feb 8, 2026

View reviewed changes

src/Appium.Net/Appium/Service/AppiumLocalService.cs Show resolved Hide resolved

Dor-bl added 2 commits February 8, 2026 20:45

Enhance service shutdown logic for OS compatibility

e398ede

Added System.Runtime.InteropServices for platform checks. Updated shutdown logic to handle Windows-specific graceful shutdown and included exception handling for robustness.

Dor-bl requested review from KazuCocoa, Copilot and mykola-mokhnach February 8, 2026 19:50

Copilot started reviewing on behalf of Dor-bl February 8, 2026 19:51 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

KazuCocoa approved these changes Feb 9, 2026

View reviewed changes

-            {
+            {
+                // Process is not running / not started; treat as exited
+                return true;
+            }
+            catch (ObjectDisposedException)
+            {

fix: Stop local appium process with SIGTERM #1006

Are you sure you want to change the base?

fix: Stop local appium process with SIGTERM #1006

Uh oh!

Conversation

Dor-bl commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

List of changes

Types of changes

Documentation

This can be done by navigating to the documentation section on http://appium.io selecting the appropriate command/endpoint and clicking the 'Edit this doc' link to update the C# example

Integration tests

Details

Uh oh!

linux-foundation-easycla bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

KazuCocoa Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

KazuCocoa left a comment

Choose a reason for hiding this comment

Uh oh!

KazuCocoa Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Dor-bl commented Feb 7, 2026 •

edited

Loading

linux-foundation-easycla bot commented Feb 7, 2026 •

edited

Loading