Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 106 additions & 1 deletion src/Appium.Net/Appium/Service/AppiumLocalService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@
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;
using System.Threading.Tasks;
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using System.Threading; appears unused in this file as-is (no Thread/WaitHandle usages). If you don't end up needing it, remove it to avoid unnecessary usings/warnings.

Suggested change
using System.Threading.Tasks;

Copilot uses AI. Check for mistakes.

namespace OpenQA.Selenium.Appium.Service
Expand All @@ -31,6 +34,23 @@ namespace OpenQA.Selenium.Appium.Service
/// </summary>
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);
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimise the use of unmanaged code.

Copilot uses AI. Check for mistakes.

[DllImport("kernel32.dll", SetLastError = true)]
private static extern bool FreeConsole();
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimise the use of unmanaged code.

Copilot uses AI. Check for mistakes.

[DllImport("kernel32.dll", SetLastError = true)]
private static extern bool SetConsoleCtrlHandler(ConsoleCtrlDelegate HandlerRoutine, bool Add);
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimise the use of unmanaged code.

Copilot uses AI. Check for mistakes.

[DllImport("kernel32.dll", SetLastError = true)]
private static extern bool GenerateConsoleCtrlEvent(uint dwCtrlEvent, uint dwProcessGroupId);
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimise the use of unmanaged code.

Copilot uses AI. Check for mistakes.

private delegate bool ConsoleCtrlDelegate(uint CtrlType);

private readonly FileInfo NodeJS;
private readonly string Arguments;
private readonly IPAddress IP;
Expand Down Expand Up @@ -144,6 +164,86 @@ private async Task StartAsync()
}
}

/// <summary>
/// Attempts to gracefully shutdown the process on Windows by sending CTRL+C signal.
/// </summary>
/// <param name="process">The process to shutdown</param>
/// <param name="timeoutMs">Timeout in milliseconds to wait for graceful shutdown</param>
/// <returns>True if process exited gracefully, false otherwise</returns>
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 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;
}
Comment on lines +188 to +196
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetConsoleCtrlHandler(null, true) is not reliably reverted: if AttachConsole fails (or GenerateConsoleCtrlEvent returns false before the re-enable call), the method returns early with Ctrl-C handling still disabled for the current process. This can leave the host test runner ignoring Ctrl+C for the rest of its lifetime. Use a try/finally (with a flag) to always restore the handler state before any return, and ensure any console attachment is cleaned up as well.

Copilot uses AI. Check for mistakes.
Comment on lines +192 to +196
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AttachConsole will fail with ERROR_ACCESS_DENIED if the current process is already attached to a console (common for dotnet test / CLI runs). Currently there is no FreeConsole() call before AttachConsole, so the graceful shutdown path will often never execute and will always fall back to Kill(). Consider calling FreeConsole() before AttachConsole (ignore failure if not attached), and restore the original console state in a finally.

Copilot uses AI. Check for mistakes.

// Send CTRL+C event
if (!GenerateConsoleCtrlEvent(CTRL_C_EVENT, 0))
{
FreeConsole();
return false;
}

// Detach from the process console
FreeConsole();

Comment on lines +198 to +207
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GenerateConsoleCtrlEvent is asynchronous. Detaching from the console immediately after generating the event can make delivery unreliable. Consider keeping the console attached briefly (e.g., a short sleep) before FreeConsole(), or otherwise ensure the event has time to be delivered before detaching.

Copilot uses AI. Check for mistakes.
// Re-enable Ctrl-C handling for our own process
SetConsoleCtrlHandler(null, false);
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Copilot uses AI. Check for mistakes.

// Wait for the process to exit gracefully
if (process.WaitForExit(timeoutMs))
{
return true;
}
Comment on lines +188 to +215
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Suggested change
// 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;
}
// Attempt to gracefully close the process using managed APIs only.
try
{
if (!process.HasExited)
{
// Request the process to exit by closing its main window (if any).
if (process.CloseMainWindow())
{
// Wait for the process to exit gracefully within the timeout.
if (process.WaitForExit(timeoutMs))
{
return true;
}
}
}
}
catch (InvalidOperationException)
{
// The process may have already exited or cannot be interacted with.
// In that case, let the caller handle any further cleanup or termination.
}

Copilot uses AI. Check for mistakes.
}
catch (Win32Exception)
{
// Windows API call failed, return false to fallback to Kill()
try
{
FreeConsole();
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Copilot uses AI. Check for mistakes.
SetConsoleCtrlHandler(null, false);
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Copilot uses AI. Check for mistakes.
}
catch (Win32Exception)
{
// Ignore cleanup errors - we're already in error handling
}
}
Comment on lines +217 to +229
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Win32 APIs invoked via P/Invoke (AttachConsole, GenerateConsoleCtrlEvent, FreeConsole, SetConsoleCtrlHandler) return bool and will not throw Win32Exception by themselves. The catch (Win32Exception) blocks therefore won’t run, and the cleanup logic inside them is effectively dead. Prefer putting cleanup/restoration in a finally and (if needed) use Marshal.GetLastWin32Error() for diagnostics when a call returns false.

Copilot uses AI. Check for mistakes.
catch (InvalidOperationException)
{
// Process has already exited, return false to fallback to Kill()
try
{
FreeConsole();
SetConsoleCtrlHandler(null, false);
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Copilot uses AI. Check for mistakes.
}
catch (Win32Exception)
{
// Ignore cleanup errors - we're already in error handling
}
Comment on lines +188 to +241
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Suggested change
// 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;
}
}
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 (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
}
// Attempt to close the main window gracefully using managed APIs
if (process.CloseMainWindow())
{
// Wait for the process to exit gracefully
if (process.WaitForExit(timeoutMs))
{
return true;
}
}
}
catch (InvalidOperationException)
{
// Process has already exited or cannot be closed this way,
// return false to fallback to Kill()

Copilot uses AI. Check for mistakes.
Comment on lines +188 to +241
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Suggested change
// 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;
}
}
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 (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
}
// Attempt a graceful shutdown by sending a close request to the main window
if (!process.CloseMainWindow())
{
// The process does not have a main window or the request could not be sent
return false;
}
// Wait for the process to exit gracefully
if (process.WaitForExit(timeoutMs))
{
return true;
}
}
catch (InvalidOperationException)
{
// Process has already exited, return false to fallback to Kill()

Copilot uses AI. Check for mistakes.
Comment on lines +188 to +241
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Suggested change
// 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;
}
}
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 (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
}
// Attempt to gracefully close the process using managed APIs
if (process.MainWindowHandle != IntPtr.Zero)
{
// Request that the main window close
if (process.CloseMainWindow())
{
// Wait for the process to exit gracefully
if (process.WaitForExit(timeoutMs))
{
return true;
}
}
}
}
catch (InvalidOperationException)
{
// Process has already exited or is not in a valid state, return false to fallback to Kill()

Copilot uses AI. Check for mistakes.
Comment on lines +188 to +241
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Suggested change
// 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;
}
}
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 (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
}
// Attempt a graceful shutdown using managed APIs by closing the main window.
// If the process does not have a main window, we cannot request a graceful exit this way.
if (process.MainWindowHandle == IntPtr.Zero)
{
return false;
}
// Request that the process close.
if (!process.CloseMainWindow())
{
return false;
}
// Wait for the process to exit gracefully.
if (process.WaitForExit(timeoutMs))
{
return true;
}
}
catch (InvalidOperationException)
{
// The process may have already exited.
return true;

Copilot uses AI. Check for mistakes.
}

Comment on lines +175 to +243
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this call with a call to managed code if possible.

Suggested change
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 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;
}
}
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 (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
}
}
if (process == null)
{
return true;
}
if (process.HasExited)
{
return true;
}
try
{
// Attempt a graceful shutdown using managed APIs only.
// CloseMainWindow sends a close request to the main window, if any.
if (process.CloseMainWindow())
{
if (process.WaitForExit(timeoutMs))
{
return true;
}
}
}
catch (InvalidOperationException)
{
// Process has already exited or is not associated with a process resource.
return true;
}
// Graceful shutdown did not succeed within the timeout.
// Caller may choose to fall back to a forceful termination.

Copilot uses AI. Check for mistakes.
return false;
}

private void DestroyProcess()
{
if (Service == null)
Expand All @@ -153,7 +253,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
{
Expand Down
Loading