Skip to content

Prevent premature end of the Benchmark process at Ctrl-C, fixes #2483 #2661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
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
22 changes: 8 additions & 14 deletions src/BenchmarkDotNet.Diagnostics.Windows/EtwDiagnoser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
using BenchmarkDotNet.Analysers;
using BenchmarkDotNet.Diagnosers;
using BenchmarkDotNet.Exporters;
using BenchmarkDotNet.Helpers;
using BenchmarkDotNet.Loggers;
using Microsoft.Diagnostics.Tracing;
using Microsoft.Diagnostics.Tracing.Parsers;
using Microsoft.Diagnostics.Tracing.Session;

namespace BenchmarkDotNet.Diagnostics.Windows
{
public abstract class EtwDiagnoser<TStats> where TStats : new()
public abstract class EtwDiagnoser<TStats> : DisposeAtProcessTermination where TStats : new()
{
internal readonly LogCapture Logger = new LogCapture();
protected readonly Dictionary<BenchmarkCase, int> BenchmarkToProcess = new Dictionary<BenchmarkCase, int>();
Expand All @@ -39,11 +40,6 @@ protected void Start(DiagnoserActionParameters parameters)
BenchmarkToProcess.Add(parameters.BenchmarkCase, parameters.Process.Id);
StatsPerProcess.TryAdd(parameters.Process.Id, GetInitializedStats(parameters));

// Important: Must wire-up clean-up events prior to acquiring IDisposable instance (Session property)
// This is in effect the inverted sequence of actions in the Stop() method.
Console.CancelKeyPress += OnConsoleCancelKeyPress;
AppDomain.CurrentDomain.ProcessExit += OnProcessExit;

Session = CreateSession(parameters.BenchmarkCase);

EnableProvider();
Expand Down Expand Up @@ -80,11 +76,13 @@ protected virtual void EnableProvider()
protected void Stop()
{
WaitForDelayedEvents();
Dispose();
}

Session.Dispose();

Console.CancelKeyPress -= OnConsoleCancelKeyPress;
AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
public override void Dispose()
{
Session?.Dispose();
base.Dispose();
}

private void Clear()
Expand All @@ -93,10 +91,6 @@ private void Clear()
StatsPerProcess.Clear();
}

private void OnConsoleCancelKeyPress(object sender, ConsoleCancelEventArgs e) => Session?.Dispose();

private void OnProcessExit(object sender, EventArgs e) => Session?.Dispose();

private static string GetSessionName(string prefix, BenchmarkCase benchmarkCase, ParameterInstances? parameters = null)
{
if (parameters != null && parameters.Items.Count > 0)
Expand Down
19 changes: 4 additions & 15 deletions src/BenchmarkDotNet.Diagnostics.Windows/EtwProfiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,10 @@ private void Start(DiagnoserActionParameters parameters)
private void Stop(DiagnoserActionParameters parameters)
{
WaitForDelayedEvents();
string userSessionFile;
try
{
kernelSession.Stop();
heapSession?.Stop();
userSession.Stop();

userSessionFile = userSession.FilePath;
}
finally
{
kernelSession.Dispose();
heapSession?.Dispose();
userSession.Dispose();
}
string userSessionFile = userSession.FilePath;
kernelSession.Dispose();
heapSession?.Dispose();
userSession.Dispose();

// Merge the 'primary' etl file X.etl (userSession) with any files that match .clr*.etl .user*.etl. and .kernel.etl.
TraceEventSession.MergeInPlace(userSessionFile, TextWriter.Null);
Expand Down
23 changes: 6 additions & 17 deletions src/BenchmarkDotNet.Diagnostics.Windows/Sessions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
using BenchmarkDotNet.Diagnosers;
using BenchmarkDotNet.Engines;
using BenchmarkDotNet.Exporters;
using BenchmarkDotNet.Extensions;
using BenchmarkDotNet.Helpers;
using BenchmarkDotNet.Loggers;
using BenchmarkDotNet.Extensions;
using BenchmarkDotNet.Running;
using Microsoft.Diagnostics.Tracing;
using Microsoft.Diagnostics.Tracing.Parsers;
using Microsoft.Diagnostics.Tracing.Session;
using BenchmarkDotNet.Running;

namespace BenchmarkDotNet.Diagnostics.Windows
{
Expand Down Expand Up @@ -90,7 +90,7 @@ internal override Session EnableProviders()
}
}

internal abstract class Session : IDisposable
internal abstract class Session : DisposeAtProcessTermination
{
private const int MaxSessionNameLength = 128;

Expand All @@ -114,27 +114,16 @@ protected Session(string sessionName, DiagnoserActionParameters details, EtwProf
BufferSizeMB = config.BufferSizeInMb,
CpuSampleIntervalMSec = config.CpuSampleIntervalInMilliseconds,
};

Console.CancelKeyPress += OnConsoleCancelKeyPress;
AppDomain.CurrentDomain.ProcessExit += OnProcessExit;
}

public void Dispose() => TraceEventSession.Dispose();

internal void Stop()
public override void Dispose()
{
TraceEventSession.Stop();

Console.CancelKeyPress -= OnConsoleCancelKeyPress;
AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
TraceEventSession.Dispose();
base.Dispose();
}

internal abstract Session EnableProviders();

private void OnConsoleCancelKeyPress(object sender, ConsoleCancelEventArgs e) => Stop();

private void OnProcessExit(object sender, EventArgs e) => Stop();

protected static string GetSessionName(BenchmarkCase benchmarkCase)
{
string benchmarkName = FullNameProvider.GetBenchmarkName(benchmarkCase);
Expand Down
57 changes: 57 additions & 0 deletions src/BenchmarkDotNet/Helpers/DisposeAtProcessTermination.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
using System;
using System.Runtime.InteropServices;

namespace BenchmarkDotNet.Helpers
{
/// <summary>
/// Ensures that explicit Dispose is called at termination of the Process.
/// </summary>
/// <remarks>
/// <para>
/// This class exists to help in reverting system state where C#'s using statement does not
/// suffice. I.e. when Benchmark's process is aborted via Ctrl-C, Ctrl-Break or via click on the
/// X in the upper right of Window.
/// </para>
/// <para>
/// Usage: Derive your clas that changes system state of this class. Revert system state in
/// override of <see cref="Dispose"/> implementation.
/// Use your class in C#'s using statement, to ensure system state is reverted in normal situations.
/// This class ensures your override is also called at process 'abort'.
/// </para>
/// <para>
/// Note: This class is explicitly not responsible for cleanup of Native resources. Of course,
/// derived classes can cleanup their Native resources (usually managed via
/// <see cref="SafeHandle"/> derived classes), by delegating explicit Disposal to their
/// <see cref="IDisposable"/> fields.
/// </para>
/// </remarks>
public abstract class DisposeAtProcessTermination : IDisposable
{
public DisposeAtProcessTermination()
{
Console.CancelKeyPress += OnCancelKeyPress;
AppDomain.CurrentDomain.ProcessExit += OnProcessExit;
// It does not make sense to include a Finalizer. We do not manage any native resource and:
// as we are subscribed to static events, it would never be called.
}

/// <summary>
/// Called when the user presses Ctrl-C or Ctrl-Break.
/// </summary>
private void OnCancelKeyPress(object? sender, ConsoleCancelEventArgs e)
{
if (!e.Cancel) { Dispose(); }
}

/// <summary>
/// Called when the user clicks on the X in the upper right corner to close the Benchmark's Window.
/// </summary>
private void OnProcessExit(object? sender, EventArgs e) => Dispose();

public virtual void Dispose()
{
Console.CancelKeyPress -= OnCancelKeyPress;
AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
}
}
}
15 changes: 4 additions & 11 deletions src/BenchmarkDotNet/Helpers/TaskbarProgress.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal enum TaskbarProgressState
Warning = Paused
}

internal class TaskbarProgress : IDisposable
internal class TaskbarProgress : DisposeAtProcessTermination
{
private static readonly bool OsVersionIsSupported = OsDetector.IsWindows()
// Must be windows 7 or greater
Expand All @@ -31,10 +31,6 @@ internal TaskbarProgress(TaskbarProgressState initialTaskbarState)
{
com = Com.MaybeCreateInstanceAndSetInitialState(initialTaskbarState);
terminal = Terminal.MaybeCreateInstanceAndSetInitialState(initialTaskbarState);
if (IsEnabled)
{
Console.CancelKeyPress += OnConsoleCancelEvent;
}
}
}

Expand All @@ -51,23 +47,20 @@ internal void SetProgress(float progressValue)
{
throw new ArgumentOutOfRangeException(nameof(progressValue), "progressValue must be between 0 and 1 inclusive.");
}
uint value = (uint) (progressValue * 100);
uint value = (uint)(progressValue * 100);
com?.SetValue(value);
terminal?.SetValue(value);
}

private void OnConsoleCancelEvent(object sender, ConsoleCancelEventArgs e)
=> Dispose();

public void Dispose()
public override void Dispose()
{
if (IsEnabled)
{
SetState(TaskbarProgressState.NoProgress);
com = null;
terminal = null;
Console.CancelKeyPress -= OnConsoleCancelEvent;
}
base.Dispose();
}

private sealed class Com
Expand Down
7 changes: 4 additions & 3 deletions src/BenchmarkDotNet/Running/ConsoleTitler.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
using System;
using System.IO;
using BenchmarkDotNet.Detectors;
using BenchmarkDotNet.Portability;
using BenchmarkDotNet.Helpers;

namespace BenchmarkDotNet.Running
{
/// <summary>
/// Updates Console.Title, subject to platform capabilities and Console availability.
/// Restores the original (or fallback) title upon disposal.
/// </summary>
internal class ConsoleTitler : IDisposable
internal class ConsoleTitler : DisposeAtProcessTermination
{
/// <summary>
/// Whether this instance has any effect. This will be false if the platform doesn't support Console retitling,
Expand Down Expand Up @@ -76,13 +76,14 @@ public void UpdateTitle(string title)
}
}

public void Dispose()
public override void Dispose()
{
if (IsEnabled)
{
Console.Title = oldConsoleTitle;
IsEnabled = false;
}
base.Dispose();
}
}
}
9 changes: 6 additions & 3 deletions src/BenchmarkDotNet/Running/PowerManagementApplier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
using BenchmarkDotNet.Environments;
using BenchmarkDotNet.Helpers;
using BenchmarkDotNet.Loggers;
using BenchmarkDotNet.Portability;

namespace BenchmarkDotNet.Running
{
internal class PowerManagementApplier : IDisposable
internal class PowerManagementApplier : DisposeAtProcessTermination
{
private static readonly Guid UserPowerPlan = new Guid("67b4a053-3646-4532-affd-0535c9ea82a7");

Expand All @@ -28,7 +27,11 @@ internal class PowerManagementApplier : IDisposable

internal PowerManagementApplier(ILogger logger) => this.logger = logger;

public void Dispose() => ApplyUserPowerPlan();
public override void Dispose()
{
ApplyUserPowerPlan();
base.Dispose();
}

internal static Guid Map(PowerPlan value) => PowerPlansDict[value];

Expand Down
Loading