Skip to content

Commit 47650ff

Browse files
brdeyobrdeyo
authored
Bug fix. Change process kill logic back to original logic for Geekbench. (#618)
* Bug fix. Change process kill logic back to original logic for Geekbench. * Fix kill logic for Prime95 as well.
1 parent d9dd817 commit 47650ff

12 files changed

Lines changed: 223 additions & 191 deletions

File tree

‎VERSION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2.1.49
1+
2.1.50

‎src/VirtualClient/VirtualClient.Actions.UnitTests/GeekBench/GeekBenchMetricsParserTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public void GeekBenchParserVerifySingleCoreResult()
3333
}
3434

3535
[Test]
36-
public void GeekBench5ParserVerifyMetricsSingleCore()
36+
public void GeekBench5ParserVerifyMetricsSingleCore_Test()
3737
{
3838
string outputPath = Path.Combine(workingDirectory, "Examples", "Geekbench", "GeekBench5Example.txt");
3939
this.rawText = File.ReadAllText(outputPath);

‎src/VirtualClient/VirtualClient.Actions/GeekBench/GeekbenchExecutor.cs‎

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace VirtualClient.Actions
55
{
66
using System;
77
using System.Collections.Generic;
8+
using System.Diagnostics;
89
using System.IO;
910
using System.IO.Abstractions;
1011
using System.Linq;
@@ -161,22 +162,20 @@ protected override async Task InitializeAsync(EventContext telemetryContext, Can
161162
{
162163
using (IProcessProxy process = this.processManager.CreateProcess(this.ExecutablePath, $"--unlock {email} {licenseKey}"))
163164
{
164-
this.CleanupTasks.Add(() =>
165+
try
165166
{
166-
// Note:
167-
// Issues were found on Linux/ARM64 systems with the process failing to be killed
168-
// using standard .NET logic. This happens on Ampere systems with lots of cores.
169-
// We are using the 'kill -9' option as a workaround.
170-
this.processManager.SafeKill(process, this.Logger);
171-
});
167+
await process.StartAndWaitAsync(cancellationToken, withExitConfirmation: true);
172168

173-
await process.StartAndWaitAsync(cancellationToken);
174-
175-
if (!cancellationToken.IsCancellationRequested)
169+
if (!cancellationToken.IsCancellationRequested)
170+
{
171+
await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName);
172+
process.ThrowIfDependencyInstallationFailed();
173+
}
174+
}
175+
finally
176176
{
177-
await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName, logToFile: true);
178-
179-
process.ThrowIfDependencyInstallationFailed();
177+
process.Close();
178+
process.SafeKill(this.Logger, TimeSpan.FromSeconds(30));
180179
}
181180
}
182181
}
@@ -213,6 +212,7 @@ private void CaptureMetrics(IProcessProxy process, string standardOutput, string
213212
// using workload name as testName
214213
GeekBenchMetricsParser geekbenchMetricsParser = new GeekBenchMetricsParser(standardOutput);
215214
IList<Metric> metrics = geekbenchMetricsParser.Parse();
215+
216216
foreach (Metric metric in metrics)
217217
{
218218
this.Logger.LogMetric(
@@ -259,24 +259,23 @@ private Task ExecuteWorkloadAsync(string pathToExe, string commandLineArguments,
259259
{
260260
using (IProcessProxy process = this.processManager.CreateProcess(pathToExe, commandLineArguments))
261261
{
262-
this.CleanupTasks.Add(() =>
262+
try
263263
{
264-
// Note:
265-
// Issues were found on Linux/ARM64 systems with the process failing to be killed
266-
// using standard .NET logic. This happens on Ampere systems with lots of cores.
267-
// We are using the 'kill -9' option as a workaround.
268-
this.processManager.SafeKill(process, this.Logger);
269-
});
264+
await process.StartAndWaitAsync(cancellationToken, withExitConfirmation: true);
270265

271-
await process.StartAndWaitAsync(cancellationToken);
266+
if (!cancellationToken.IsCancellationRequested)
267+
{
268+
await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName);
269+
process.ThrowIfWorkloadFailed();
272270

273-
if (!cancellationToken.IsCancellationRequested)
271+
string standardOutput = process.StandardOutput.ToString();
272+
this.CaptureMetrics(process, standardOutput, commandLineArguments, telemetryContext, cancellationToken);
273+
}
274+
}
275+
finally
274276
{
275-
await this.LogProcessDetailsAsync(process, telemetryContext, this.PackageName, logToFile: true);
276-
process.ThrowIfWorkloadFailed();
277-
278-
string standardOutput = process.StandardOutput.ToString();
279-
this.CaptureMetrics(process, standardOutput, commandLineArguments, telemetryContext, cancellationToken);
277+
process.Close();
278+
process.SafeKill(this.Logger, TimeSpan.FromSeconds(30));
280279
}
281280
}
282281
}

‎src/VirtualClient/VirtualClient.Actions/Prime95/Prime95Executor.cs‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -293,39 +293,43 @@ await this.Logger.LogMessageAsync($"{this.TypeName}.ExecuteWorkload", telemetryC
293293
{
294294
using (IProcessProxy process = this.systemManagement.ProcessManager.CreateProcess(this.ExecutablePath, commandArguments, this.WorkloadPackage.Path))
295295
{
296-
this.CleanupTasks.Add(() => process.SafeKill());
297-
298-
// Prime95 does not stop on it's own. It will run until you tell it to stop.
299-
// We have to definitively stop the program.
300296
DateTime explicitTimeout = DateTime.UtcNow.Add(this.Duration);
301297

302298
if (process.Start())
303299
{
304-
await this.WaitAsync(explicitTimeout, cancellationToken);
305-
process.SafeKill();
306-
307-
if (!cancellationToken.IsCancellationRequested)
300+
try
308301
{
309-
KeyValuePair<string, string> results = default;
302+
await this.WaitAsync(explicitTimeout, cancellationToken);
310303

311-
if (this.fileSystem.File.Exists(this.ResultsFilePath))
304+
if (!cancellationToken.IsCancellationRequested)
312305
{
313-
await RetryPolicies.FileOperations.ExecuteAsync(async () =>
306+
KeyValuePair<string, string> results = default;
307+
308+
if (this.fileSystem.File.Exists(this.ResultsFilePath))
314309
{
315-
results = await this.LoadResultsAsync(this.ResultsFilePath, cancellationToken);
316-
});
317-
}
310+
await RetryPolicies.FileOperations.ExecuteAsync(async () =>
311+
{
312+
results = await this.LoadResultsAsync(this.ResultsFilePath, cancellationToken);
313+
});
314+
}
318315

319-
await this.LogProcessDetailsAsync(process, telemetryContext, "Prime95", results: results);
316+
await this.LogProcessDetailsAsync(process, telemetryContext, "Prime95", results: results);
320317

321-
// The exit code on SafeKill is -1 which is not a part of the default success codes.
322-
process.ThrowIfWorkloadFailed(this.successExitCodes);
318+
// The exit code on SafeKill is -1 which is not a part of the default success codes.
319+
process.ThrowIfWorkloadFailed(this.successExitCodes);
323320

324-
if (!string.IsNullOrWhiteSpace(results.Value))
325-
{
326-
this.CaptureMetrics(process, results.Value, telemetryContext, cancellationToken);
321+
if (!string.IsNullOrWhiteSpace(results.Value))
322+
{
323+
this.CaptureMetrics(process, results.Value, telemetryContext, cancellationToken);
324+
}
327325
}
328326
}
327+
finally
328+
{
329+
// Prime95 does not stop on it's own. It will run until you tell it to stop.
330+
// We have to definitively stop the program.
331+
process.SafeKill(this.Logger);
332+
}
329333
}
330334
}
331335
});

‎src/VirtualClient/VirtualClient.Contracts/Parser/TextParsingExtensions.cs‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public static class TextParsingExtensions
5959
public static readonly string EmailRegex = @"[\w\-\.]+@([\w -]+\.)+[\w-]{2,}";
6060

6161
private static readonly Regex CommaDelimitedExpression = new Regex(",", RegexOptions.Compiled | RegexOptions.IgnoreCase);
62+
private static readonly Regex NewLineExpression = new Regex("\r\n|\n", RegexOptions.Compiled | RegexOptions.IgnoreCase);
6263
private static readonly Regex SemiColonDelimitedExpression = new Regex(";", RegexOptions.Compiled | RegexOptions.IgnoreCase);
6364
private static readonly Regex TripleCommaDelimitedExpression = new Regex(",,,", RegexOptions.Compiled | RegexOptions.IgnoreCase);
6465

@@ -70,7 +71,7 @@ public static class TextParsingExtensions
7071
public static string RemoveRows(string text, Regex delimiter)
7172
{
7273
List<string> result = new List<string>();
73-
List<string> rows = text.Split(Environment.NewLine, StringSplitOptions.None).ToList();
74+
List<string> rows = NewLineExpression.Split(text).ToList();
7475

7576
foreach (string row in rows)
7677
{
@@ -91,17 +92,25 @@ public static string RemoveRows(string text, Regex delimiter)
9192
public static IDictionary<string, string> Sectionize(string text, Regex delimiter)
9293
{
9394
IDictionary<string, string> result = new Dictionary<string, string>();
94-
string[] sections = Regex.Split(text, delimiter.ToString(), delimiter.Options);
95+
IEnumerable<string> sections = Regex.Split(text, delimiter.ToString(), delimiter.Options)
96+
.Where(s => !string.IsNullOrWhiteSpace(s?.Trim()));
9597

96-
foreach (string section in sections)
98+
if (sections?.Any() == true)
9799
{
98-
if (!string.IsNullOrWhiteSpace(section))
100+
foreach (string section in sections)
99101
{
100-
List<string> rows = section.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries).ToList();
101-
string sectionName = rows.FirstOrDefault().Trim();
102-
rows.RemoveAt(0);
103-
104-
result.Add(sectionName, string.Join(Environment.NewLine, rows.Select(r => r.Trim())));
102+
string sectionInfo = section?.Trim();
103+
if (!string.IsNullOrWhiteSpace(sectionInfo))
104+
{
105+
List<string> rows = NewLineExpression.Split(sectionInfo).ToList();
106+
if (rows?.Any() == true)
107+
{
108+
string sectionName = rows.FirstOrDefault().Trim();
109+
rows.RemoveAt(0);
110+
111+
result.Add(sectionName, string.Join(Environment.NewLine, rows.Select(r => r.Trim())));
112+
}
113+
}
105114
}
106115
}
107116

@@ -389,7 +398,7 @@ public static string TranslateStorageByUnit(string text, string metricUnit)
389398
public static IDictionary<string, string> Sectionize(string text, IDictionary<string, Regex> matchingRegexes)
390399
{
391400
IDictionary<string, string> result = new Dictionary<string, string>();
392-
List<string> rows = text.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries).ToList();
401+
List<string> rows = NewLineExpression.Split(text).ToList();
393402

394403
foreach (KeyValuePair<string, Regex> section in matchingRegexes)
395404
{

‎src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,7 @@ public async Task ExecuteAsync(CancellationToken cancellationToken)
708708

709709
try
710710
{
711+
this.CleanupTasks.Clear();
711712
PlatformSpecifics.ThrowIfNotSupported(this.Platform);
712713
PlatformSpecifics.ThrowIfNotSupported(this.CpuArchitecture);
713714

@@ -786,9 +787,8 @@ await this.Logger.LogMessageAsync($"{this.TypeName}.Execute", telemetryContext,
786787
{
787788
this.EndTime = DateTime.UtcNow;
788789
this.LogSuccessOrFailedMetric(succeeded, scenarioStartTime: this.StartTime, scenarioEndTime: this.EndTime, telemetryContext: telemetryContext);
790+
await this.CleanupAsync(telemetryContext, cancellationToken);
789791
}
790-
791-
await this.CleanupAsync(telemetryContext, cancellationToken);
792792
});
793793
}
794794
}
@@ -804,31 +804,32 @@ await this.Logger.LogMessageAsync($"{this.TypeName}.Execute", telemetryContext,
804804
/// </summary>
805805
protected virtual Task CleanupAsync(EventContext telemetryContext, CancellationToken cancellationToken)
806806
{
807-
if (this.CleanupTasks.Any())
807+
return Task.Run(() =>
808808
{
809-
try
809+
if (this.CleanupTasks.Any())
810810
{
811-
foreach (Action cleanupTask in this.CleanupTasks)
811+
try
812812
{
813-
try
814-
{
815-
cleanupTask.Invoke();
816-
}
817-
catch (Exception exc)
813+
foreach (Action cleanupTask in this.CleanupTasks)
818814
{
819-
// Best effort...but logged
820-
this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc));
815+
try
816+
{
817+
cleanupTask.Invoke();
818+
}
819+
catch (Exception exc)
820+
{
821+
// Best effort...but logged
822+
this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc));
823+
}
821824
}
822825
}
826+
catch (Exception exc)
827+
{
828+
// Best effort...but logged
829+
this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc));
830+
}
823831
}
824-
catch (Exception exc)
825-
{
826-
// Best effort...but logged
827-
this.Logger.LogMessage($"{this.TypeName}.CleanupError", LogLevel.Warning, telemetryContext.Clone().AddError(exc));
828-
}
829-
}
830-
831-
return Task.CompletedTask;
832+
});
832833
}
833834

834835
/// <summary>

0 commit comments

Comments
 (0)