diff --git a/PowerToys.slnx b/PowerToys.slnx index e6b34c2baa..fd647cf82d 100644 --- a/PowerToys.slnx +++ b/PowerToys.slnx @@ -495,6 +495,10 @@ + + + + @@ -1284,4 +1288,3 @@ - diff --git a/doc/devdocs/modules/filelocksmith.md b/doc/devdocs/modules/filelocksmith.md index a678617821..16c43016e2 100644 --- a/doc/devdocs/modules/filelocksmith.md +++ b/doc/devdocs/modules/filelocksmith.md @@ -46,8 +46,10 @@ The module adds "Unlock with File Locksmith" to the context menu in File Explore 3. The shell extension writes the selected file path to a temporary file (file-based IPC) 4. The shell extension launches `PowerToys.FileLocksmithUI.exe` 5. The UI reads the file path from the temporary file -6. The UI uses `FileLocksmithLibInterop` to scan for processes with handles to the file -7. Results are displayed in the UI, showing process information and allowing user action +6. The UI starts `FileLocksmithCLI.exe` in a hidden worker mode and sends the selected paths over redirected standard input +7. The worker uses `FileLocksmithLib` to scan for processes with handles to the file and returns JSON over redirected standard output +8. The UI terminates the worker process tree if the scan exceeds 30 seconds or is canceled, and reports the failure instead of displaying an empty result +9. Results are displayed in the UI, showing process information and allowing user action ### Core Functionality @@ -57,6 +59,7 @@ The core functionality to find processes locking files is implemented in [FileLo - Examines all running processes to find handles to the specified files - Maps process IDs to the files they're locking - Retrieves process information such as name, user context, and file paths +- Runs outside the UI process because `NtQueryObject` and `GetFileType` can block indefinitely for individual handles ### User Interface diff --git a/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.cpp b/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.cpp index 39d9fec0d2..787803078d 100644 --- a/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.cpp +++ b/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.cpp @@ -79,6 +79,13 @@ std::wstring get_json(const std::vector& results) return root.Stringify().c_str(); } +CommandResult run_worker_query(const std::vector& paths, IProcessFinder& finder) +{ + auto results = finder.find(paths); + Logger::info("Worker enumeration found {} processes", results.size()); + return { 0, get_json(results), L"worker-query" }; +} + std::wstring get_text(const std::vector& results, IStringProvider& strings) { std::wstringstream ss; diff --git a/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.h b/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.h index eba9003c36..58ce2dce70 100644 --- a/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.h +++ b/src/modules/FileLocksmith/FileLocksmithCLI/CLILogic.h @@ -30,3 +30,4 @@ struct IStringProvider }; CommandResult run_command(int argc, wchar_t* argv[], IProcessFinder& finder, IProcessTerminator& terminator, IStringProvider& strings); +CommandResult run_worker_query(const std::vector& paths, IProcessFinder& finder); diff --git a/src/modules/FileLocksmith/FileLocksmithCLI/main.cpp b/src/modules/FileLocksmith/FileLocksmithCLI/main.cpp index 15a8c4b4a1..eeeae3a702 100644 --- a/src/modules/FileLocksmith/FileLocksmithCLI/main.cpp +++ b/src/modules/FileLocksmith/FileLocksmithCLI/main.cpp @@ -2,7 +2,11 @@ #include "CLILogic.h" #include "FileLocksmithLib/FileLocksmith.h" #include "FileLocksmithLib/Trace.h" +#include +#include #include +#include +#include #include "resource.h" #include #include @@ -44,6 +48,53 @@ struct RealStringProvider : IStringProvider } }; +namespace +{ + constexpr std::wstring_view WorkerArgument = L"--worker-json"; + + std::optional> read_worker_paths() + { + const std::string input{ + std::istreambuf_iterator{ std::cin }, + std::istreambuf_iterator{} + }; + + json::JsonObject request; + if (!json::JsonObject::TryParse(winrt::to_hstring(input), request) || !request.HasKey(L"paths")) + { + return std::nullopt; + } + + try + { + std::vector paths; + const auto json_paths = request.GetNamedArray(L"paths"); + paths.reserve(json_paths.Size()); + + for (const auto& path : json_paths) + { + if (path.ValueType() != json::JsonValueType::String) + { + return std::nullopt; + } + + paths.emplace_back(path.GetString()); + } + + if (paths.empty()) + { + return std::nullopt; + } + + return paths; + } + catch (const winrt::hresult_error&) + { + return std::nullopt; + } + } +} + #ifndef UNIT_TEST int wmain(int argc, wchar_t* argv[]) { @@ -56,6 +107,29 @@ int wmain(int argc, wchar_t* argv[]) RealProcessTerminator terminator; RealStringProvider strings; + if (argc == 2 && argv[1] == WorkerArgument) + { + const auto paths = read_worker_paths(); + if (!paths) + { + Logger::error("Worker input was malformed"); + Trace::CLICommand(L"worker-query", false); + Trace::UnregisterProvider(); + return 2; + } + + Logger::info("Worker query started with {} paths", paths->size()); + const auto started = std::chrono::steady_clock::now(); + const auto result = run_worker_query(*paths, finder); + const auto duration = std::chrono::duration_cast(std::chrono::steady_clock::now() - started); + Logger::info("Worker query completed in {} ms with exit code {}", duration.count(), result.exit_code); + + std::cout << winrt::to_string(result.output); + Trace::CLICommand(result.command_name.c_str(), result.exit_code == 0); + Trace::UnregisterProvider(); + return result.exit_code; + } + auto result = run_command(argc, argv, finder, terminator, strings); if (result.exit_code != 0) diff --git a/src/modules/FileLocksmith/FileLocksmithLibInterop/FileLocksmith.cpp b/src/modules/FileLocksmith/FileLocksmithLibInterop/FileLocksmith.cpp index 069a95bd37..ae500321c2 100644 --- a/src/modules/FileLocksmith/FileLocksmithLibInterop/FileLocksmith.cpp +++ b/src/modules/FileLocksmith/FileLocksmithLibInterop/FileLocksmith.cpp @@ -117,6 +117,10 @@ constexpr size_t LongMaxPathSize = 65536; std::wstring pid_to_full_path(DWORD pid) { HANDLE process = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid); + if (!process) + { + return {}; + } std::wstring result(LongMaxPathSize, L'\0'); diff --git a/src/modules/FileLocksmith/FileLocksmithLibInterop/NtdllExtensions.cpp b/src/modules/FileLocksmith/FileLocksmithLibInterop/NtdllExtensions.cpp index e95cd16124..d3f64ecb8f 100644 --- a/src/modules/FileLocksmith/FileLocksmithLibInterop/NtdllExtensions.cpp +++ b/src/modules/FileLocksmith/FileLocksmithLibInterop/NtdllExtensions.cpp @@ -1,8 +1,6 @@ #include "pch.h" #include "NtdllExtensions.h" -#include -#include #define STATUS_INFO_LENGTH_MISMATCH ((LONG)0xC0000004) @@ -12,11 +10,21 @@ namespace { std::wstring_view unicode_to_view(UNICODE_STRING unicode_str) { + if (!unicode_str.Buffer || unicode_str.Length == 0) + { + return {}; + } + return std::wstring_view(unicode_str.Buffer, unicode_str.Length / sizeof(WCHAR)); } std::wstring unicode_to_str(UNICODE_STRING unicode_str) { + if (!unicode_str.Buffer || unicode_str.Length == 0) + { + return {}; + } + return std::wstring(unicode_str.Buffer, unicode_str.Length / sizeof(WCHAR)); } @@ -164,119 +172,66 @@ std::vector NtdllExtensions::handles() noexcept std::map pid_to_handle; std::vector result; - std::vector object_info_buffer(DefaultResultBufferSize); - std::atomic i = 0; - std::atomic handle_count = info_ptr->NumberOfHandles; - std::atomic process_handle = NULL; - std::atomic handle_copy = NULL; - ULONG previous_i; - - - while (i < handle_count) + const ULONG_PTR handle_count = info_ptr->NumberOfHandles; + for (ULONG_PTR i = 0; i < handle_count; i++) { - previous_i = i; + const auto* handle_info = info_ptr->Handles + i; + const auto pid = handle_info->UniqueProcessId; - // The system calls we use in this block were reported to hang on some machines. - // We need to offload the cycle to another thread and keep track of progress to terminate and resume when needed. - // Unfortunately, there are no alternative APIs to what we're using that accept timeouts. (NtQueryObject and GetFileType) - auto offload_function = std::thread([&] { - for (; i < handle_count; i++) - { - process_handle = NULL; - handle_copy = NULL; - - auto handle_info = info_ptr->Handles + i; - auto pid = handle_info->UniqueProcessId; - - auto iter = pid_to_handle.find(pid); - if (iter != pid_to_handle.end()) - { - process_handle = iter->second; - } - else - { - process_handle = OpenProcess(PROCESS_DUP_HANDLE, FALSE, static_cast(pid)); - if (!process_handle) - { - continue; - } - pid_to_handle[pid] = process_handle; - } - - // According to this: - // https://stackoverflow.com/questions/46384048/enumerate-handles - // NtQueryObject could hang - - // TODO uncomment and investigate - // if (handle_info->GrantedAccess == 0x0012019f) { - // continue; - // } - - HANDLE local_handle_copy; - auto dh_result = DuplicateHandle(process_handle, reinterpret_cast(handle_info->HandleValue), GetCurrentProcess(), &local_handle_copy, 0, 0, DUPLICATE_SAME_ACCESS); - if (dh_result == 0) - { - // Ignore this handle. - continue; - } - handle_copy = local_handle_copy; - - ULONG return_length; - auto status = NtQueryObject(handle_copy, ObjectTypeInformation, object_info_buffer.data(), static_cast(object_info_buffer.size()), &return_length); - if (NT_ERROR(status)) - { - // Ignore this handle. - CloseHandle(handle_copy); - handle_copy = NULL; - continue; - } - - auto object_type_info = reinterpret_cast(object_info_buffer.data()); - auto type_name = unicode_to_str(object_type_info->Name); - - std::wstring file_name; - - if (type_name == L"File") - { - file_name = file_handle_to_kernel_name(handle_copy, object_info_buffer); - result.push_back(HandleInfo{ pid, handle_info->HandleValue, type_name, file_name }); - } - - CloseHandle(handle_copy); - handle_copy = NULL; - } - }); - - offload_function.detach(); - do + HANDLE process_handle = NULL; + if (auto iter = pid_to_handle.find(pid); iter != pid_to_handle.end()) { - Sleep(200); // Timeout in milliseconds for detecting that the system hang on getting information for a handle. - if (i >= handle_count) + process_handle = iter->second; + } + else + { + process_handle = OpenProcess(PROCESS_DUP_HANDLE, FALSE, static_cast(pid)); + if (!process_handle) { - // We're done. - break; + continue; } - if (previous_i >= i) - { - // The thread looks like it's hanging on some handle. Let's kill it and resume. + pid_to_handle[pid] = process_handle; + } - // HACK: This is unsafe and may leak something, but looks like there's no way to properly clean up a thread when it's hanging on a system call. - TerminateThread(offload_function.native_handle(), 1); + HANDLE handle_copy = NULL; + if (!DuplicateHandle( + process_handle, + reinterpret_cast(handle_info->HandleValue), + GetCurrentProcess(), + &handle_copy, + 0, + FALSE, + DUPLICATE_SAME_ACCESS)) + { + continue; + } - // Close Handles that might be lingering. - if (handle_copy!=NULL) - { - CloseHandle(handle_copy); - } - i++; - break; - } - previous_i = i; - } while (1); + ULONG return_length = 0; + const auto status = NtQueryObject( + handle_copy, + ObjectTypeInformation, + object_info_buffer.data(), + static_cast(object_info_buffer.size()), + &return_length); + if (NT_ERROR(status)) + { + CloseHandle(handle_copy); + continue; + } + auto object_type_info = reinterpret_cast(object_info_buffer.data()); + auto type_name = unicode_to_str(object_type_info->Name); + + if (type_name == L"File") + { + auto file_name = file_handle_to_kernel_name(handle_copy, object_info_buffer); + result.push_back(HandleInfo{ pid, handle_info->HandleValue, std::move(type_name), std::move(file_name) }); + } + + CloseHandle(handle_copy); } for (auto [pid, handle] : pid_to_handle) diff --git a/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithUI.csproj b/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithUI.csproj index f6633cbf62..1a08725b43 100644 --- a/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithUI.csproj +++ b/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithUI.csproj @@ -84,6 +84,10 @@ + + false + true + diff --git a/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/MainWindow.xaml.cs b/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/MainWindow.xaml.cs index 6557dc13b2..64854cc684 100644 --- a/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/MainWindow.xaml.cs +++ b/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/MainWindow.xaml.cs @@ -36,10 +36,18 @@ namespace FileLocksmithUI Title = title; titleBar.Title = title; + Closed += MainWindow_Closed; } public void Dispose() { + Closed -= MainWindow_Closed; + mainPage.ViewModel.Dispose(); + } + + private void MainWindow_Closed(object sender, WindowEventArgs args) + { + Dispose(); } } } diff --git a/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/Views/MainPage.xaml b/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/Views/MainPage.xaml index 3dffe8c46c..bc5573c22a 100644 --- a/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/Views/MainPage.xaml +++ b/src/modules/FileLocksmith/FileLocksmithUI/FileLocksmithXAML/Views/MainPage.xaml @@ -38,6 +38,7 @@ + @@ -84,7 +85,15 @@ - + + + - + Processes, + int? ExitCode = null); +} diff --git a/src/modules/FileLocksmith/FileLocksmithUI/Services/FileLocksmithQueryService.cs b/src/modules/FileLocksmith/FileLocksmithUI/Services/FileLocksmithQueryService.cs new file mode 100644 index 0000000000..90c9645bec --- /dev/null +++ b/src/modules/FileLocksmith/FileLocksmithUI/Services/FileLocksmithQueryService.cs @@ -0,0 +1,253 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Diagnostics; +using System.IO; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Threading; +using System.Threading.Tasks; + +namespace PowerToys.FileLocksmithUI.Services +{ + internal sealed class FileLocksmithQueryService + { + internal static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(30); + + private const string WorkerExecutableName = "FileLocksmithCLI.exe"; + private const string WorkerArgument = "--worker-json"; + + private readonly Func _startInfoFactory; + private readonly TimeSpan _timeout; + private readonly Action? _processStarted; + + internal FileLocksmithQueryService() + : this(CreateWorkerStartInfo, DefaultTimeout, null) + { + } + + internal FileLocksmithQueryService( + Func startInfoFactory, + TimeSpan timeout, + Action? processStarted) + { + ArgumentNullException.ThrowIfNull(startInfoFactory); + + ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(timeout, TimeSpan.Zero); + + _startInfoFactory = startInfoFactory; + _timeout = timeout; + _processStarted = processStarted; + } + + internal async Task FindProcessesAsync( + IReadOnlyCollection paths, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(paths); + + if (paths.Count == 0) + { + return new FileLocksmithQueryResult(FileLocksmithQueryStatus.Success, Array.Empty()); + } + + using var process = new Process + { + StartInfo = _startInfoFactory(), + }; + + try + { + if (!process.Start()) + { + return FailedToStart(); + } + } + catch (Win32Exception) + { + return FailedToStart(); + } + catch (InvalidOperationException) + { + return FailedToStart(); + } + + _processStarted?.Invoke(process.Id); + +#pragma warning disable CA2016 // These reads must drain the redirected pipes after timeout cancellation. + var outputTask = process.StandardOutput.ReadToEndAsync(CancellationToken.None); + var errorTask = process.StandardError.ReadToEndAsync(CancellationToken.None); +#pragma warning restore CA2016 + var inputClosed = false; + + try + { + using var timeoutCancellation = new CancellationTokenSource(_timeout); + using var linkedCancellation = CancellationTokenSource.CreateLinkedTokenSource( + timeoutCancellation.Token, + cancellationToken); + + try + { + var request = JsonSerializer.Serialize(new WorkerRequest(paths)); + await process.StandardInput.WriteAsync(request.AsMemory(), linkedCancellation.Token); + process.StandardInput.Close(); + inputClosed = true; + + await process.WaitForExitAsync(linkedCancellation.Token); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + await TerminateAsync(process); + await Task.WhenAll(outputTask, errorTask); + return new FileLocksmithQueryResult( + FileLocksmithQueryStatus.TimedOut, + Array.Empty()); + } + catch (IOException) + { + return Failed(process.HasExited ? process.ExitCode : null); + } + catch (InvalidOperationException) + { + return Failed(process.HasExited ? process.ExitCode : null); + } + + if (process.ExitCode != 0) + { + await Task.WhenAll(outputTask, errorTask); + return Failed(process.ExitCode); + } + + var output = await outputTask; + await errorTask; + + try + { + var response = JsonSerializer.Deserialize(output); + if (response?.Processes is null) + { + return MalformedOutput(); + } + + var processes = new List(response.Processes.Length); + foreach (var processInfo in response.Processes) + { + if (processInfo.Name is null || + processInfo.User is null || + processInfo.Files is null) + { + return MalformedOutput(); + } + + processes.Add(new FileLocksmithProcessInfo( + processInfo.Name, + processInfo.Pid, + processInfo.User, + processInfo.Files)); + } + + return new FileLocksmithQueryResult( + FileLocksmithQueryStatus.Success, + processes); + } + catch (JsonException) + { + return MalformedOutput(); + } + } + finally + { + if (!process.HasExited) + { + await TerminateAsync(process); + } + + if (!inputClosed) + { + process.StandardInput.BaseStream.Dispose(); + } + + await Task.WhenAll(outputTask, errorTask); + } + } + + private static ProcessStartInfo CreateWorkerStartInfo() + { + var installedPath = Path.Combine(AppContext.BaseDirectory, WorkerExecutableName); + var buildOutputPath = Path.GetFullPath(Path.Combine(AppContext.BaseDirectory, "..", WorkerExecutableName)); + var workerPath = File.Exists(installedPath) ? installedPath : buildOutputPath; + + var startInfo = new ProcessStartInfo(workerPath) + { + CreateNoWindow = true, + RedirectStandardError = true, + RedirectStandardInput = true, + RedirectStandardOutput = true, + StandardErrorEncoding = Encoding.UTF8, + StandardInputEncoding = Encoding.UTF8, + StandardOutputEncoding = Encoding.UTF8, + UseShellExecute = false, + }; + startInfo.ArgumentList.Add(WorkerArgument); + return startInfo; + } + + private static async Task TerminateAsync(Process process) + { + if (process.HasExited) + { + return; + } + + try + { + process.Kill(entireProcessTree: true); + } + catch (InvalidOperationException) when (process.HasExited) + { + } + + await process.WaitForExitAsync(CancellationToken.None); + } + + private static FileLocksmithQueryResult FailedToStart() => + Failed(null); + + private static FileLocksmithQueryResult Failed(int? exitCode) => + new(FileLocksmithQueryStatus.Failed, Array.Empty(), exitCode); + + private static FileLocksmithQueryResult MalformedOutput() => + new(FileLocksmithQueryStatus.MalformedOutput, Array.Empty()); + + private sealed record WorkerRequest([property: JsonPropertyName("paths")] IReadOnlyCollection Paths); + + private sealed class WorkerResponse + { + [JsonPropertyName("processes")] + public WorkerProcessInfo[]? Processes { get; init; } + } + + private sealed class WorkerProcessInfo + { + [JsonPropertyName("name")] + public string? Name { get; init; } + + [JsonPropertyName("pid")] + public uint Pid { get; init; } + + [JsonPropertyName("user")] + public string? User { get; init; } + + [JsonPropertyName("files")] + public string[]? Files { get; init; } + } + } +} diff --git a/src/modules/FileLocksmith/FileLocksmithUI/Services/FileLocksmithQueryStatus.cs b/src/modules/FileLocksmith/FileLocksmithUI/Services/FileLocksmithQueryStatus.cs new file mode 100644 index 0000000000..d8be475f68 --- /dev/null +++ b/src/modules/FileLocksmith/FileLocksmithUI/Services/FileLocksmithQueryStatus.cs @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace PowerToys.FileLocksmithUI.Services +{ + internal enum FileLocksmithQueryStatus + { + Success, + TimedOut, + Failed, + MalformedOutput, + } +} diff --git a/src/modules/FileLocksmith/FileLocksmithUI/Strings/en-us/Resources.resw b/src/modules/FileLocksmith/FileLocksmithUI/Strings/en-us/Resources.resw index 12f5278295..755d5d22fd 100644 --- a/src/modules/FileLocksmith/FileLocksmithUI/Strings/en-us/Resources.resw +++ b/src/modules/FileLocksmith/FileLocksmithUI/Strings/en-us/Resources.resw @@ -124,6 +124,14 @@ No results + + File Locksmith couldn't scan the selected files. Select Reload to try again. + Error shown when the isolated process used to scan files fails or returns invalid data. + + + File Locksmith couldn't finish scanning within 30 seconds. Select Reload to try again. + Error shown when the isolated process used to scan files exceeds its time limit. + End task diff --git a/src/modules/FileLocksmith/FileLocksmithUI/ViewModels/MainViewModel.cs b/src/modules/FileLocksmith/FileLocksmithUI/ViewModels/MainViewModel.cs index 7a0db51c00..93caee4461 100644 --- a/src/modules/FileLocksmith/FileLocksmithUI/ViewModels/MainViewModel.cs +++ b/src/modules/FileLocksmith/FileLocksmithUI/ViewModels/MainViewModel.cs @@ -3,10 +3,9 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; -using System.Linq; +using System.Globalization; using System.Threading; using System.Threading.Tasks; @@ -14,6 +13,8 @@ using CommunityToolkit.Mvvm.ComponentModel; using CommunityToolkit.Mvvm.Input; using ManagedCommon; using PowerToys.FileLocksmithLib.Interop; +using PowerToys.FileLocksmithUI.Helpers; +using PowerToys.FileLocksmithUI.Services; namespace PowerToys.FileLocksmithUI.ViewModels { @@ -23,14 +24,30 @@ namespace PowerToys.FileLocksmithUI.ViewModels { public IAsyncRelayCommand LoadProcessesCommand { get; } + private readonly FileLocksmithQueryService _queryService = new(); private bool _isLoading; private bool _isElevated; private string[] paths; private bool _disposed; private CancellationTokenSource _cancelProcessWatching; + private CancellationTokenSource _cancelQuery; + private string _queryErrorMessage; public ObservableCollection Processes { get; } = new(); + public string QueryErrorMessage + { + get => _queryErrorMessage; + private set + { + _queryErrorMessage = value; + OnPropertyChanged(nameof(QueryErrorMessage)); + OnPropertyChanged(nameof(HasQueryError)); + } + } + + public bool HasQueryError => !string.IsNullOrEmpty(QueryErrorMessage); + public bool IsLoading { get @@ -87,36 +104,51 @@ namespace PowerToys.FileLocksmithUI.ViewModels private async Task LoadProcessesAsync() { IsLoading = true; + QueryErrorMessage = null; Processes.Clear(); - if (_cancelProcessWatching is not null) - { - _cancelProcessWatching.Cancel(); - } - + _cancelProcessWatching?.Cancel(); + _cancelProcessWatching?.Dispose(); _cancelProcessWatching = new CancellationTokenSource(); - var processes_found = await FindProcesses(paths); - if (processes_found is not null) + _cancelQuery?.Cancel(); + _cancelQuery?.Dispose(); + _cancelQuery = new CancellationTokenSource(); + var cancellationToken = _cancelQuery.Token; + var stopwatch = Stopwatch.StartNew(); + + try { - foreach (ProcessResult p in processes_found) + var queryResult = await _queryService.FindProcessesAsync(paths, cancellationToken); + stopwatch.Stop(); + + if (queryResult.Status == FileLocksmithQueryStatus.Success) { - Processes.Add(p); - WatchProcess(p, _cancelProcessWatching.Token); + Logger.LogInfo($"File Locksmith worker query completed in {stopwatch.ElapsedMilliseconds} ms with exit code 0 and {queryResult.Processes.Count} processes."); + foreach (var processInfo in queryResult.Processes) + { + var process = new ProcessResult(processInfo.Name, processInfo.Pid, processInfo.User, processInfo.Files); + Processes.Add(process); + WatchProcess(process, _cancelProcessWatching.Token); + } + } + else + { + var exitCode = queryResult.ExitCode?.ToString(CultureInfo.InvariantCulture) ?? "unavailable"; + Logger.LogError($"File Locksmith worker query failed at stage enumeration after {stopwatch.ElapsedMilliseconds} ms with status {queryResult.Status} and exit code {exitCode}."); + QueryErrorMessage = queryResult.Status == FileLocksmithQueryStatus.TimedOut + ? ResourceLoaderInstance.ResourceLoader.GetString("QueryTimeoutError") + : ResourceLoaderInstance.ResourceLoader.GetString("QueryFailedError"); } } - - IsLoading = false; - } - - private async Task> FindProcesses(string[] paths) - { - var results = new List(); - await Task.Run(() => + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - results = NativeMethods.FindProcessesRecursive(paths)?.ToList(); - }); - return results; + Logger.LogInfo($"File Locksmith worker query canceled at stage enumeration after {stopwatch.ElapsedMilliseconds} ms."); + } + finally + { + IsLoading = false; + } } private async void WatchProcess(ProcessResult process, CancellationToken token) @@ -194,6 +226,10 @@ namespace PowerToys.FileLocksmithUI.ViewModels { if (disposing) { + _cancelQuery?.Cancel(); + _cancelQuery?.Dispose(); + _cancelProcessWatching?.Cancel(); + _cancelProcessWatching?.Dispose(); _disposed = true; } } diff --git a/src/modules/FileLocksmith/Tests/FileLocksmithUI.UnitTests/FileLocksmithQueryServiceTests.cs b/src/modules/FileLocksmith/Tests/FileLocksmithUI.UnitTests/FileLocksmithQueryServiceTests.cs new file mode 100644 index 0000000000..013b952a5b --- /dev/null +++ b/src/modules/FileLocksmith/Tests/FileLocksmithUI.UnitTests/FileLocksmithQueryServiceTests.cs @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +using PowerToys.FileLocksmithUI.Services; + +namespace PowerToys.FileLocksmithUI.UnitTests +{ + [TestClass] + public sealed class FileLocksmithQueryServiceTests + { + private static readonly string[] SelectedPaths = [@"C:\file.txt"]; + + [TestMethod] + public async Task FindProcessesAsyncReturnsWorkerResults() + { + const string output = """ + {"processes":[{"pid":123,"name":"process.exe","user":"user","files":["C:\\file.txt"]}]} + """; + var service = CreateService($"[Console]::Out.Write('{output}')"); + + var result = await service.FindProcessesAsync(SelectedPaths, CancellationToken.None); + + Assert.AreEqual(FileLocksmithQueryStatus.Success, result.Status); + Assert.HasCount(1, result.Processes); + Assert.AreEqual(123U, result.Processes[0].Pid); + Assert.AreEqual("process.exe", result.Processes[0].Name); + Assert.AreEqual(@"C:\file.txt", result.Processes[0].Files[0]); + } + + [TestMethod] + public async Task FindProcessesAsyncReportsMalformedWorkerOutput() + { + var service = CreateService("[Console]::Out.Write('not-json')"); + + var result = await service.FindProcessesAsync(SelectedPaths, CancellationToken.None); + + Assert.AreEqual(FileLocksmithQueryStatus.MalformedOutput, result.Status); + Assert.IsEmpty(result.Processes); + } + + [TestMethod] + public async Task FindProcessesAsyncReportsFailedWorkerExitCode() + { + var service = CreateService("exit 17"); + + var result = await service.FindProcessesAsync(SelectedPaths, CancellationToken.None); + + Assert.AreEqual(FileLocksmithQueryStatus.Failed, result.Status); + Assert.AreEqual(17, result.ExitCode); + Assert.IsEmpty(result.Processes); + } + + [TestMethod] + public async Task FindProcessesAsyncTimeoutTerminatesWorker() + { + var workerPid = 0; + var service = CreateService( + "Start-Sleep -Seconds 30", + TimeSpan.FromMilliseconds(500), + pid => workerPid = pid); + + var result = await service.FindProcessesAsync(SelectedPaths, CancellationToken.None); + + Assert.AreEqual(FileLocksmithQueryStatus.TimedOut, result.Status); + Assert.AreNotEqual(0, workerPid); + Assert.IsFalse(IsProcessRunning(workerPid), "The timed-out worker process was left running."); + } + + private static FileLocksmithQueryService CreateService( + string script, + TimeSpan? timeout = null, + Action? processStarted = null) + { + return new FileLocksmithQueryService( + () => + { + var startInfo = new ProcessStartInfo("powershell.exe") + { + CreateNoWindow = true, + RedirectStandardError = true, + RedirectStandardInput = true, + RedirectStandardOutput = true, + UseShellExecute = false, + }; + startInfo.ArgumentList.Add("-NoLogo"); + startInfo.ArgumentList.Add("-NoProfile"); + startInfo.ArgumentList.Add("-NonInteractive"); + startInfo.ArgumentList.Add("-Command"); + startInfo.ArgumentList.Add($"$null = [Console]::In.ReadToEnd(); {script}"); + return startInfo; + }, + timeout ?? TimeSpan.FromSeconds(10), + processStarted); + } + + private static bool IsProcessRunning(int pid) + { + try + { + using var process = Process.GetProcessById(pid); + return !process.HasExited; + } + catch (ArgumentException) + { + return false; + } + } + } +} diff --git a/src/modules/FileLocksmith/Tests/FileLocksmithUI.UnitTests/FileLocksmithUI.UnitTests.csproj b/src/modules/FileLocksmith/Tests/FileLocksmithUI.UnitTests/FileLocksmithUI.UnitTests.csproj new file mode 100644 index 0000000000..8ea4f7a2c6 --- /dev/null +++ b/src/modules/FileLocksmith/Tests/FileLocksmithUI.UnitTests/FileLocksmithUI.UnitTests.csproj @@ -0,0 +1,30 @@ + + + + + + true + PowerToys.FileLocksmithUI.UnitTests + true + win-x64 + win-arm64 + false + false + $(RepoRoot)$(Platform)\$(Configuration)\tests\FileLocksmithUI.UnitTests\ + enable + false + Exe + + + + + + + + + + + + + +