From 4b19de07e573ac3d4a816741ca7ede97cbde5afd Mon Sep 17 00:00:00 2001 From: brdeyo Date: Thu, 15 Jan 2026 10:05:00 -0800 Subject: [PATCH 1/2] Bug fix. Change process kill logic back to original logic for Geekbench. --- .../GeekBench/GeekBenchMetricsParserTests.cs | 10 ++ .../GeekBench/GeekbenchExecutor.cs | 53 ++++--- .../Parser/TextParsingExtensions.cs | 29 ++-- .../VirtualClientComponent.cs | 41 +++--- .../VirtualClientRuntime.cs | 132 +++++++++++------- .../VirtualClient.Core/DependencyFactory.cs | 17 +-- .../Logging/MetricsCsvFileLogger.cs | 6 +- .../Logging/MetricsCsvFileLoggerProvider.cs | 8 +- .../VirtualClient.Core/ProcessExtensions.cs | 61 ++++++++ .../ExecuteProfileCommand.cs | 11 +- 10 files changed, 235 insertions(+), 133 deletions(-) diff --git a/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs b/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs index 6401818344..503509f858 100644 --- a/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs +++ b/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs @@ -34,6 +34,16 @@ public void GeekBenchParserVerifySingleCoreResult() [Test] public void GeekBench5ParserVerifyMetricsSingleCore() + { + string outputPath = Path.Combine(workingDirectory, "Examples", "Geekbench", "GeekBench5Example.txt"); + this.rawText = File.ReadAllText("C:\\Users\\bryan\\Downloads\\virtualclient.2.1.3201.1956\\content\\linux-x64\\logs\\geekbench6\\2026-01-12-195804648912-executegeekbench6benchmark.log"); + + this.testParser = new GeekBenchMetricsParser(this.rawText); + IList metrics = this.testParser.Parse(); + } + + [Test] + public void GeekBench5ParserVerifyMetricsSingleCore_Test() { string outputPath = Path.Combine(workingDirectory, "Examples", "Geekbench", "GeekBench5Example.txt"); this.rawText = File.ReadAllText(outputPath); diff --git a/src/VirtualClient/VirtualClient.Actions/GeekBench/GeekbenchExecutor.cs b/src/VirtualClient/VirtualClient.Actions/GeekBench/GeekbenchExecutor.cs index fa9cdf49ec..cf6455ecd9 100644 --- a/src/VirtualClient/VirtualClient.Actions/GeekBench/GeekbenchExecutor.cs +++ b/src/VirtualClient/VirtualClient.Actions/GeekBench/GeekbenchExecutor.cs @@ -5,6 +5,7 @@ namespace VirtualClient.Actions { using System; using System.Collections.Generic; + using System.Diagnostics; using System.IO; using System.IO.Abstractions; using System.Linq; @@ -161,22 +162,20 @@ protected override async Task InitializeAsync(EventContext telemetryContext, Can { using (IProcessProxy process = this.processManager.CreateProcess(this.ExecutablePath, $"--unlock {email} {licenseKey}")) { - this.CleanupTasks.Add(() => + try { - // Note: - // Issues were found on Linux/ARM64 systems with the process failing to be killed - // using standard .NET logic. This happens on Ampere systems with lots of cores. - // We are using the 'kill -9' option as a workaround. - this.processManager.SafeKill(process, this.Logger); - }); + await process.StartAndWaitAsync(cancellationToken, withExitConfirmation: true); - await process.StartAndWaitAsync(cancellationToken); - - if (!cancellationToken.IsCancellationRequested) + if (!cancellationToken.IsCancellationRequested) + { + await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName); + process.ThrowIfDependencyInstallationFailed(); + } + } + finally { - await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName, logToFile: true); - - process.ThrowIfDependencyInstallationFailed(); + process.Close(); + process.SafeKill(this.Logger, TimeSpan.FromSeconds(30)); } } } @@ -213,6 +212,7 @@ private void CaptureMetrics(IProcessProxy process, string standardOutput, string // using workload name as testName GeekBenchMetricsParser geekbenchMetricsParser = new GeekBenchMetricsParser(standardOutput); IList metrics = geekbenchMetricsParser.Parse(); + foreach (Metric metric in metrics) { this.Logger.LogMetric( @@ -259,24 +259,23 @@ private Task ExecuteWorkloadAsync(string pathToExe, string commandLineArguments, { using (IProcessProxy process = this.processManager.CreateProcess(pathToExe, commandLineArguments)) { - this.CleanupTasks.Add(() => + try { - // Note: - // Issues were found on Linux/ARM64 systems with the process failing to be killed - // using standard .NET logic. This happens on Ampere systems with lots of cores. - // We are using the 'kill -9' option as a workaround. - this.processManager.SafeKill(process, this.Logger); - }); + await process.StartAndWaitAsync(cancellationToken, withExitConfirmation: true); - await process.StartAndWaitAsync(cancellationToken); + if (!cancellationToken.IsCancellationRequested) + { + await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName); + process.ThrowIfWorkloadFailed(); - if (!cancellationToken.IsCancellationRequested) + string standardOutput = process.StandardOutput.ToString(); + this.CaptureMetrics(process, standardOutput, commandLineArguments, telemetryContext, cancellationToken); + } + } + finally { - await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName, logToFile: true); - process.ThrowIfWorkloadFailed(); - - string standardOutput = process.StandardOutput.ToString(); - this.CaptureMetrics(process, standardOutput, commandLineArguments, telemetryContext, cancellationToken); + process.Close(); + process.SafeKill(this.Logger, TimeSpan.FromSeconds(30)); } } } diff --git a/src/VirtualClient/VirtualClient.Contracts/Parser/TextParsingExtensions.cs b/src/VirtualClient/VirtualClient.Contracts/Parser/TextParsingExtensions.cs index bbb6cf8e02..66491ad4a8 100644 --- a/src/VirtualClient/VirtualClient.Contracts/Parser/TextParsingExtensions.cs +++ b/src/VirtualClient/VirtualClient.Contracts/Parser/TextParsingExtensions.cs @@ -59,6 +59,7 @@ public static class TextParsingExtensions public static readonly string EmailRegex = @"[\w\-\.]+@([\w -]+\.)+[\w-]{2,}"; private static readonly Regex CommaDelimitedExpression = new Regex(",", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly Regex NewLineExpression = new Regex("\r\n|\n", RegexOptions.Compiled | RegexOptions.IgnoreCase); private static readonly Regex SemiColonDelimitedExpression = new Regex(";", RegexOptions.Compiled | RegexOptions.IgnoreCase); private static readonly Regex TripleCommaDelimitedExpression = new Regex(",,,", RegexOptions.Compiled | RegexOptions.IgnoreCase); @@ -70,7 +71,7 @@ public static class TextParsingExtensions public static string RemoveRows(string text, Regex delimiter) { List result = new List(); - List rows = text.Split(Environment.NewLine, StringSplitOptions.None).ToList(); + List rows = NewLineExpression.Split(text).ToList(); foreach (string row in rows) { @@ -91,17 +92,25 @@ public static string RemoveRows(string text, Regex delimiter) public static IDictionary Sectionize(string text, Regex delimiter) { IDictionary result = new Dictionary(); - string[] sections = Regex.Split(text, delimiter.ToString(), delimiter.Options); + IEnumerable sections = Regex.Split(text, delimiter.ToString(), delimiter.Options) + .Where(s => !string.IsNullOrWhiteSpace(s?.Trim())); - foreach (string section in sections) + if (sections?.Any() == true) { - if (!string.IsNullOrWhiteSpace(section)) + foreach (string section in sections) { - List rows = section.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries).ToList(); - string sectionName = rows.FirstOrDefault().Trim(); - rows.RemoveAt(0); - - result.Add(sectionName, string.Join(Environment.NewLine, rows.Select(r => r.Trim()))); + string sectionInfo = section?.Trim(); + if (!string.IsNullOrWhiteSpace(sectionInfo)) + { + List rows = NewLineExpression.Split(sectionInfo).ToList(); + if (rows?.Any() == true) + { + string sectionName = rows.FirstOrDefault().Trim(); + rows.RemoveAt(0); + + result.Add(sectionName, string.Join(Environment.NewLine, rows.Select(r => r.Trim()))); + } + } } } @@ -389,7 +398,7 @@ public static string TranslateStorageByUnit(string text, string metricUnit) public static IDictionary Sectionize(string text, IDictionary matchingRegexes) { IDictionary result = new Dictionary(); - List rows = text.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries).ToList(); + List rows = NewLineExpression.Split(text).ToList(); foreach (KeyValuePair section in matchingRegexes) { diff --git a/src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs b/src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs index 9832e8e0fe..f4028be9f1 100644 --- a/src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs +++ b/src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs @@ -708,6 +708,7 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) try { + this.CleanupTasks.Clear(); PlatformSpecifics.ThrowIfNotSupported(this.Platform); PlatformSpecifics.ThrowIfNotSupported(this.CpuArchitecture); @@ -786,9 +787,8 @@ await this.Logger.LogMessageAsync($"{this.TypeName}.Execute", telemetryContext, { this.EndTime = DateTime.UtcNow; this.LogSuccessOrFailedMetric(succeeded, scenarioStartTime: this.StartTime, scenarioEndTime: this.EndTime, telemetryContext: telemetryContext); + await this.CleanupAsync(telemetryContext, cancellationToken); } - - await this.CleanupAsync(telemetryContext, cancellationToken); }); } } @@ -804,31 +804,32 @@ await this.Logger.LogMessageAsync($"{this.TypeName}.Execute", telemetryContext, /// protected virtual Task CleanupAsync(EventContext telemetryContext, CancellationToken cancellationToken) { - if (this.CleanupTasks.Any()) + return Task.Run(() => { - try + if (this.CleanupTasks.Any()) { - foreach (Action cleanupTask in this.CleanupTasks) + try { - try - { - cleanupTask.Invoke(); - } - catch (Exception exc) + foreach (Action cleanupTask in this.CleanupTasks) { - // Best effort...but logged - this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc)); + try + { + cleanupTask.Invoke(); + } + catch (Exception exc) + { + // Best effort...but logged + this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc)); + } } } + catch (Exception exc) + { + // Best effort...but logged + this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc)); + } } - catch (Exception exc) - { - // Best effort...but logged - this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc)); - } - } - - return Task.CompletedTask; + }); } /// diff --git a/src/VirtualClient/VirtualClient.Contracts/VirtualClientRuntime.cs b/src/VirtualClient/VirtualClient.Contracts/VirtualClientRuntime.cs index 8a2352de0a..9d335f8875 100644 --- a/src/VirtualClient/VirtualClient.Contracts/VirtualClientRuntime.cs +++ b/src/VirtualClient/VirtualClient.Contracts/VirtualClientRuntime.cs @@ -7,9 +7,11 @@ namespace VirtualClient using System.Collections.Generic; using System.Diagnostics; using System.Linq; + using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json.Linq; + using VirtualClient.Common.Telemetry; using VirtualClient.Contracts; /// @@ -22,6 +24,8 @@ public static class VirtualClientRuntime /// public static readonly object LockObject = new object(); + private static readonly IList FlushableDataChannels = new List(); + /// /// Event is fired anytime special instructions are received by a component within a Virtual /// Client instance or from a remote instance. @@ -68,14 +72,15 @@ public static class VirtualClientRuntime public static IReadOnlyDictionary CommandLineParameters { get; internal set; } /// - /// The current platform-specifics for the application. + /// The set of flushable logging channels. The application must ensure all logging is flushed before + /// exiting to avoid data/telemetry loss. /// - public static PlatformSpecifics PlatformSpecifics { get; internal set; } + public static List DataChannels { get; } = new List(); /// /// The name of the Virtual Client application/module. /// - public static string ExecutableName { get; internal set; } = Process.GetCurrentProcess().MainModule.FileName; + public static string ExecutableName { get; } = Process.GetCurrentProcess().MainModule.FileName; /// /// A set of one or more tasks (exit) registered to execute before the application @@ -96,52 +101,9 @@ public static class VirtualClientRuntime public static bool IsRebootRequested { get; set; } /// - /// Cleans up any tracked resources. + /// The current platform-specifics for the application. /// - public static void OnCleanup() - { - if (VirtualClientRuntime.CleanupTasks.Any()) - { - lock (VirtualClientRuntime.LockObject) - { - foreach (var entry in VirtualClientRuntime.CleanupTasks) - { - try - { - entry.Invoke(); - } - catch - { - // Best effort here. - } - } - } - } - } - - /// - /// Cleans up any tracked resources. - /// - public static void OnExiting() - { - if (VirtualClientRuntime.ExitTasks.Any()) - { - lock (VirtualClientRuntime.LockObject) - { - foreach (var entry in VirtualClientRuntime.ExitTasks) - { - try - { - entry.Invoke(); - } - catch - { - // Best effort here. - } - } - } - } - } + public static PlatformSpecifics PlatformSpecifics { get; } = new PlatformSpecifics(Environment.OSVersion.Platform, RuntimeInformation.ProcessArchitecture); /// /// Invokes the event to notify subscribers @@ -218,5 +180,79 @@ public static void SetEventingApiOnline(bool online) VirtualClientRuntime.IsApiOnline = online; } } + + /// + /// Cleans up any tracked resources. + /// + internal static void ExecuteCleanupActions() + { + if (VirtualClientRuntime.CleanupTasks.Any()) + { + lock (VirtualClientRuntime.LockObject) + { + foreach (var entry in VirtualClientRuntime.CleanupTasks) + { + try + { + entry.Invoke(); + } + catch + { + // Best effort here. + } + } + } + } + } + + /// + /// Executes any actions before application immediate exit. + /// + internal static void ExecuteExitActions() + { + if (VirtualClientRuntime.ExitTasks.Any()) + { + lock (VirtualClientRuntime.LockObject) + { + foreach (var entry in VirtualClientRuntime.ExitTasks) + { + try + { + entry.Invoke(); + } + catch + { + // Best effort here. + } + } + } + } + } + + /// + /// Executes flush actions before application exit. + /// + /// A timeout to apply to each of the data channel flush operations. + internal static void ExecuteFlushActions(TimeSpan flushTimeout) + { + if (VirtualClientRuntime.FlushableDataChannels.Any()) + { + lock (VirtualClientRuntime.LockObject) + { + foreach (IFlushableChannel dataChannel in VirtualClientRuntime.FlushableDataChannels) + { + try + { + dataChannel.Flush(flushTimeout); + } + catch (Exception exc) + { + // Best effort here. + Console.WriteLine($"Data channel flush error: {exc.Message}"); + } + } + } + } + } } } diff --git a/src/VirtualClient/VirtualClient.Core/DependencyFactory.cs b/src/VirtualClient/VirtualClient.Core/DependencyFactory.cs index 54c98122c3..b6e828774f 100644 --- a/src/VirtualClient/VirtualClient.Core/DependencyFactory.cs +++ b/src/VirtualClient/VirtualClient.Core/DependencyFactory.cs @@ -31,8 +31,6 @@ namespace VirtualClient /// public static class DependencyFactory { - private static List telemetryChannels = new List(); - /// /// Creates an instance that can be used to download blobs/files from a store or /// upload blobs/files to a store. @@ -152,7 +150,7 @@ public static EventHubTelemetryChannel CreateEventHubTelemetryChannel(string eve EventHubTelemetryChannel channel = new EventHubTelemetryChannel(client, enableDiagnostics: true); - DependencyFactory.telemetryChannels.Add(channel); + VirtualClientRuntime.DataChannels.Add(channel); VirtualClientRuntime.CleanupTasks.Add(new Action_(() => channel.Dispose())); return channel; } @@ -585,7 +583,7 @@ public static ProxyTelemetryChannel CreateProxyTelemetryChannel(IProxyApiClient }; } - DependencyFactory.telemetryChannels.Add(channel); + VirtualClientRuntime.DataChannels.Add(channel); return channel; } @@ -700,17 +698,6 @@ public static VirtualClientProxyApiClient CreateVirtualClientProxyApiClient(Uri return new VirtualClientProxyApiClient(builder.Build(), proxyApiUri); } - /// - /// Flushes buffered telemetry from all channels. - /// - /// The absolute timeout to flush the telemetry from each individual channel. - /// - /// - public static void FlushTelemetry(TimeSpan? timeout = null) - { - Parallel.ForEach(DependencyFactory.telemetryChannels, channel => channel.Flush(timeout)); - } - /// /// Applies a filter to the logger generated by the provider that will handle the logging /// of test metrics/results events. diff --git a/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLogger.cs b/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLogger.cs index 4ddfa9be27..ea4ff89917 100644 --- a/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLogger.cs +++ b/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLogger.cs @@ -160,10 +160,11 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except this.flushTask = this.MonitorBufferAsync(); } } - catch + catch (Exception exc) { // Best effort. We do not want to crash the application on failures to access // the CSV file. + Console.WriteLine(exc.Message); } } } @@ -305,10 +306,11 @@ private Task MonitorBufferAsync() await Task.Delay(300); await this.FlushBufferAsync(); } - catch + catch (Exception exc) { // Best effort. We do not want to crash the application on failures to access // the CSV file. + Console.WriteLine(exc.Message); } } }); diff --git a/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLoggerProvider.cs b/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLoggerProvider.cs index b1446d2154..9fd083ea7c 100644 --- a/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLoggerProvider.cs +++ b/src/VirtualClient/VirtualClient.Core/Logging/MetricsCsvFileLoggerProvider.cs @@ -44,11 +44,9 @@ public MetricsCsvFileLoggerProvider(string csvFilePath, long maximumFileSizeByte public ILogger CreateLogger(string categoryName) { MetricsCsvFileLogger logger = new MetricsCsvFileLogger(this.filePath, this.maxFileSize); - VirtualClientRuntime.CleanupTasks.Add(new Action_(() => - { - logger.Flush(); - logger.Dispose(); - })); + + VirtualClientRuntime.DataChannels.Add(logger); + VirtualClientRuntime.CleanupTasks.Add(new Action_(() => logger.Dispose())); return logger; } diff --git a/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs b/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs index f24a929f4a..183beccbba 100644 --- a/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs +++ b/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs @@ -158,6 +158,67 @@ public static void SafeKill(this ProcessManager processManager, IEnumerable + /// Uses pkill on Linux systems to kill the associated process and/or its child/dependent processes. + /// + /// Provides functionality for creating processes. + /// The ID of the process to kill. + /// The logger to use to write trace information. + /// Max duration to wait for exit. Default = 10 seconds. Use TimeSpan.Zero for no wait. + public static void SafeKill(this ProcessManager processManager, int processId, ILogger logger = null, TimeSpan? confirmationWaitTime = null) + { + try + { + if (processId > 0) + { + if (processManager.Platform == PlatformID.Unix) + { + using (IProcessProxy kill = processManager.CreateProcess("kill", $"-9 {processId}")) + { + kill.StartAndWaitAsync(CancellationToken.None, confirmationWaitTime) + .GetAwaiter().GetResult(); + + // 0 = Success + // 1 = Process not found + if (kill.ExitCode != 0 && kill.ExitCode != 1) + { + kill.ThrowErrored( + $"Unix kill -9 attempt failed with exit code {kill.ExitCode} for process (id={processId}, timeout={confirmationWaitTime?.ToString() ?? "none"}). " + + $"{kill.StandardOutput} {kill.StandardError}".Trim(), + ErrorReason.WorkloadUnexpectedAnomaly); + } + } + } + else if (processManager.Platform == PlatformID.Win32NT) + { + using (IProcessProxy taskkill = processManager.CreateProcess("taskkill", $"/F /PID {processId}")) + { + taskkill.StartAndWaitAsync(CancellationToken.None, confirmationWaitTime) + .GetAwaiter().GetResult(); + + // 0 = Success + // 1 = Process not found + if (taskkill.ExitCode != 0 && taskkill.ExitCode != 128) + { + taskkill.ThrowErrored( + $"Windows taskkill attempt failed with exit code {taskkill.ExitCode} for process (id={processId}, timeout={confirmationWaitTime?.ToString() ?? "none"}). " + + $"{taskkill.StandardOutput} {taskkill.StandardError}".Trim(), + ErrorReason.WorkloadUnexpectedAnomaly); + } + } + } + } + } + catch (Exception exc) + { + EventContext errorContext = EventContext.Persisted(); + errorContext.AddContext(nameof(processId), processId); + errorContext.AddError(exc); + + logger?.LogMessage($"ProcessKillFailed", LogLevel.Warning, errorContext); + } + } + /// /// Uses pkill on Linux systems to kill the associated process and/or its child/dependent processes. /// diff --git a/src/VirtualClient/VirtualClient.Main/ExecuteProfileCommand.cs b/src/VirtualClient/VirtualClient.Main/ExecuteProfileCommand.cs index 5e917f73d0..7cf427f644 100644 --- a/src/VirtualClient/VirtualClient.Main/ExecuteProfileCommand.cs +++ b/src/VirtualClient/VirtualClient.Main/ExecuteProfileCommand.cs @@ -189,7 +189,7 @@ public override async Task ExecuteAsync(string[] args, CancellationTokenSou EventContext exitingContext = EventContext.Persisted(); // Allow components to handle any final exit operations. - VirtualClientRuntime.OnExiting(); + VirtualClientRuntime.ExecuteExitActions(); if (VirtualClientRuntime.IsRebootRequested) { @@ -211,21 +211,20 @@ public override async Task ExecuteAsync(string[] args, CancellationTokenSou } Program.LogMessage(logger, $"Flush Telemetry", exitingContext); - DependencyFactory.FlushTelemetry(remainingWait); + VirtualClientRuntime.ExecuteFlushActions(TimeSpan.FromSeconds(30)); Program.LogMessage(logger, $"Flushed", exitingContext); - DependencyFactory.FlushTelemetry(TimeSpan.FromMinutes(1)); + VirtualClientRuntime.ExecuteFlushActions(TimeSpan.FromMinutes(1)); // Allow components to handle any final cleanup operations. - VirtualClientRuntime.OnCleanup(); + VirtualClientRuntime.ExecuteCleanupActions(); // Reboots must happen after telemetry is flushed and just before the application is exiting. This ensures // we capture all important telemetry and allow the profile execution operations to exit gracefully before // we suddenly reboot the system. if (VirtualClientRuntime.IsRebootRequested) { - await CommandBase.RebootSystemAsync(dependencies) - .ConfigureAwait(false); + await CommandBase.RebootSystemAsync(dependencies); } } From 78c5f4d245adf7f268b6e718cd073e57a0b73ac6 Mon Sep 17 00:00:00 2001 From: brdeyo Date: Thu, 15 Jan 2026 10:13:24 -0800 Subject: [PATCH 2/2] Fix kill logic for Prime95 as well. --- VERSION | 2 +- .../GeekBench/GeekBenchMetricsParserTests.cs | 10 --- .../Prime95/Prime95Executor.cs | 44 +++++++------ .../VirtualClient.Core/ProcessExtensions.cs | 64 ------------------- 4 files changed, 25 insertions(+), 95 deletions(-) diff --git a/VERSION b/VERSION index f1a2ac01c3..98d723348b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.1.49 \ No newline at end of file +2.1.50 \ No newline at end of file diff --git a/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs b/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs index 503509f858..2ac249ae5d 100644 --- a/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs +++ b/src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs @@ -32,16 +32,6 @@ public void GeekBenchParserVerifySingleCoreResult() Assert.AreEqual(5, this.testParser.SingleCoreResult.Columns.Count); } - [Test] - public void GeekBench5ParserVerifyMetricsSingleCore() - { - string outputPath = Path.Combine(workingDirectory, "Examples", "Geekbench", "GeekBench5Example.txt"); - this.rawText = File.ReadAllText("C:\\Users\\bryan\\Downloads\\virtualclient.2.1.3201.1956\\content\\linux-x64\\logs\\geekbench6\\2026-01-12-195804648912-executegeekbench6benchmark.log"); - - this.testParser = new GeekBenchMetricsParser(this.rawText); - IList metrics = this.testParser.Parse(); - } - [Test] public void GeekBench5ParserVerifyMetricsSingleCore_Test() { diff --git a/src/VirtualClient/VirtualClient.Actions/Prime95/Prime95Executor.cs b/src/VirtualClient/VirtualClient.Actions/Prime95/Prime95Executor.cs index 70ad827a5f..a10eab5925 100644 --- a/src/VirtualClient/VirtualClient.Actions/Prime95/Prime95Executor.cs +++ b/src/VirtualClient/VirtualClient.Actions/Prime95/Prime95Executor.cs @@ -293,39 +293,43 @@ await this.Logger.LogMessageAsync($"{this.TypeName}.ExecuteWorkload", telemetryC { using (IProcessProxy process = this.systemManagement.ProcessManager.CreateProcess(this.ExecutablePath, commandArguments, this.WorkloadPackage.Path)) { - this.CleanupTasks.Add(() => process.SafeKill()); - - // Prime95 does not stop on it's own. It will run until you tell it to stop. - // We have to definitively stop the program. DateTime explicitTimeout = DateTime.UtcNow.Add(this.Duration); if (process.Start()) { - await this.WaitAsync(explicitTimeout, cancellationToken); - process.SafeKill(); - - if (!cancellationToken.IsCancellationRequested) + try { - KeyValuePair results = default; + await this.WaitAsync(explicitTimeout, cancellationToken); - if (this.fileSystem.File.Exists(this.ResultsFilePath)) + if (!cancellationToken.IsCancellationRequested) { - await RetryPolicies.FileOperations.ExecuteAsync(async () => + KeyValuePair results = default; + + if (this.fileSystem.File.Exists(this.ResultsFilePath)) { - results = await this.LoadResultsAsync(this.ResultsFilePath, cancellationToken); - }); - } + await RetryPolicies.FileOperations.ExecuteAsync(async () => + { + results = await this.LoadResultsAsync(this.ResultsFilePath, cancellationToken); + }); + } - await this.LogProcessDetailsAsync(process, telemetryContext, "Prime95", results: results); + await this.LogProcessDetailsAsync(process, telemetryContext, "Prime95", results: results); - // The exit code on SafeKill is -1 which is not a part of the default success codes. - process.ThrowIfWorkloadFailed(this.successExitCodes); + // The exit code on SafeKill is -1 which is not a part of the default success codes. + process.ThrowIfWorkloadFailed(this.successExitCodes); - if (!string.IsNullOrWhiteSpace(results.Value)) - { - this.CaptureMetrics(process, results.Value, telemetryContext, cancellationToken); + if (!string.IsNullOrWhiteSpace(results.Value)) + { + this.CaptureMetrics(process, results.Value, telemetryContext, cancellationToken); + } } } + finally + { + // Prime95 does not stop on it's own. It will run until you tell it to stop. + // We have to definitively stop the program. + process.SafeKill(this.Logger); + } } } }); diff --git a/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs b/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs index 183beccbba..fd720d2c02 100644 --- a/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs +++ b/src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs @@ -219,70 +219,6 @@ public static void SafeKill(this ProcessManager processManager, int processId, I } } - /// - /// Uses pkill on Linux systems to kill the associated process and/or its child/dependent processes. - /// - /// Provides functionality for creating processes. - /// The process to kill. - /// The logger to use to write trace information. - /// Max duration to wait for exit. Default = 10 seconds. Use TimeSpan.Zero for no wait. - public static void SafeKill(this ProcessManager processManager, IProcessProxy process, ILogger logger = null, TimeSpan? confirmationWaitTime = null) - { - string processName = null; - int processId = -1; - - try - { - processName = SafeGet(() => process.Name); - processId = SafeGet(() => process.Id); - - if (processManager.Platform == PlatformID.Unix) - { - using (IProcessProxy kill = processManager.CreateProcess("kill", $"-9 {processId}")) - { - kill.StartAndWaitAsync(CancellationToken.None, confirmationWaitTime) - .GetAwaiter().GetResult(); - - // 0 = Success - // 1 = Process not found - if (kill.ExitCode != 0 && kill.ExitCode != 1) - { - kill.ThrowErrored( - $"Unix kill -9 attempt failed with exit code {kill.ExitCode} for process (id={processId}, name={processName}, timeout={confirmationWaitTime?.ToString() ?? "none"}). " + - $"{kill.StandardOutput} {kill.StandardError}".Trim(), - ErrorReason.WorkloadUnexpectedAnomaly); - } - } - } - else if (processManager.Platform == PlatformID.Win32NT) - { - using (IProcessProxy taskkill = processManager.CreateProcess("taskkill", $"/F /PID {processId}")) - { - taskkill.StartAndWaitAsync(CancellationToken.None, confirmationWaitTime) - .GetAwaiter().GetResult(); - - // 0 = Success - // 1 = Process not found - if (taskkill.ExitCode != 0 && taskkill.ExitCode != 128) - { - taskkill.ThrowErrored( - $"Windows taskkill attempt failed with exit code {taskkill.ExitCode} for process (id={processId}, name={processName}, timeout={confirmationWaitTime?.ToString() ?? "none"}). " + - $"{taskkill.StandardOutput} {taskkill.StandardError}".Trim(), - ErrorReason.WorkloadUnexpectedAnomaly); - } - } - } - } - catch (Exception exc) - { - EventContext errorContext = EventContext.Persisted(); - errorContext.AddProcessDetails(process.ToProcessDetails(processName)); - errorContext.AddError(exc); - - logger?.LogMessage($"ProcessKillFailed.{processName}", LogLevel.Warning, errorContext); - } - } - /// /// Throws an exception when an action, dependency installation or monitor process has exited and the exit code does not match /// one of the default success exit codes.