Skip to content

Commit c545c08

Browse files
Ensure revert of system state at Benchmark Process termination, fixes dotnet#2483
* PowerManagementApplier and ConsoleTitler system state is now reverted at Process termination. * To prevent code duplication, DisposeAtProcessTermination class is introduced.
1 parent 346bbab commit c545c08

File tree

7 files changed

+69
-63
lines changed

7 files changed

+69
-63
lines changed

src/BenchmarkDotNet.Diagnostics.Windows/EtwDiagnoser.cs

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@
88
using BenchmarkDotNet.Analysers;
99
using BenchmarkDotNet.Diagnosers;
1010
using BenchmarkDotNet.Exporters;
11+
using BenchmarkDotNet.Helpers;
1112
using BenchmarkDotNet.Loggers;
1213
using Microsoft.Diagnostics.Tracing;
1314
using Microsoft.Diagnostics.Tracing.Parsers;
1415
using Microsoft.Diagnostics.Tracing.Session;
1516

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

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

4945
EnableProvider();
@@ -80,11 +76,13 @@ protected virtual void EnableProvider()
8076
protected void Stop()
8177
{
8278
WaitForDelayedEvents();
79+
Dispose();
80+
}
8381

84-
Session.Dispose();
85-
86-
Console.CancelKeyPress -= OnConsoleCancelKeyPress;
87-
AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
82+
public override void Dispose()
83+
{
84+
Session?.Dispose();
85+
base.Dispose();
8886
}
8987

9088
private void Clear()
@@ -93,10 +91,6 @@ private void Clear()
9391
StatsPerProcess.Clear();
9492
}
9593

96-
private void OnConsoleCancelKeyPress(object sender, ConsoleCancelEventArgs e) => Session?.Dispose();
97-
98-
private void OnProcessExit(object sender, EventArgs e) => Session?.Dispose();
99-
10094
private static string GetSessionName(string prefix, BenchmarkCase benchmarkCase, ParameterInstances? parameters = null)
10195
{
10296
if (parameters != null && parameters.Items.Count > 0)

src/BenchmarkDotNet.Diagnostics.Windows/EtwProfiler.cs

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,21 +120,10 @@ private void Start(DiagnoserActionParameters parameters)
120120
private void Stop(DiagnoserActionParameters parameters)
121121
{
122122
WaitForDelayedEvents();
123-
string userSessionFile;
124-
try
125-
{
126-
kernelSession.Stop();
127-
heapSession?.Stop();
128-
userSession.Stop();
129-
130-
userSessionFile = userSession.FilePath;
131-
}
132-
finally
133-
{
134-
kernelSession.Dispose();
135-
heapSession?.Dispose();
136-
userSession.Dispose();
137-
}
123+
string userSessionFile = userSession.FilePath;
124+
kernelSession.Dispose();
125+
heapSession?.Dispose();
126+
userSession.Dispose();
138127

139128
// Merge the 'primary' etl file X.etl (userSession) with any files that match .clr*.etl .user*.etl. and .kernel.etl.
140129
TraceEventSession.MergeInPlace(userSessionFile, TextWriter.Null);

src/BenchmarkDotNet.Diagnostics.Windows/Sessions.cs

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
using BenchmarkDotNet.Diagnosers;
66
using BenchmarkDotNet.Engines;
77
using BenchmarkDotNet.Exporters;
8+
using BenchmarkDotNet.Extensions;
89
using BenchmarkDotNet.Helpers;
910
using BenchmarkDotNet.Loggers;
10-
using BenchmarkDotNet.Extensions;
11+
using BenchmarkDotNet.Running;
1112
using Microsoft.Diagnostics.Tracing;
1213
using Microsoft.Diagnostics.Tracing.Parsers;
1314
using Microsoft.Diagnostics.Tracing.Session;
14-
using BenchmarkDotNet.Running;
1515

1616
namespace BenchmarkDotNet.Diagnostics.Windows
1717
{
@@ -90,7 +90,7 @@ internal override Session EnableProviders()
9090
}
9191
}
9292

93-
internal abstract class Session : IDisposable
93+
internal abstract class Session : DisposeAtProcessTermination
9494
{
9595
private const int MaxSessionNameLength = 128;
9696

@@ -114,27 +114,16 @@ protected Session(string sessionName, DiagnoserActionParameters details, EtwProf
114114
BufferSizeMB = config.BufferSizeInMb,
115115
CpuSampleIntervalMSec = config.CpuSampleIntervalInMilliseconds,
116116
};
117-
118-
Console.CancelKeyPress += OnConsoleCancelKeyPress;
119-
AppDomain.CurrentDomain.ProcessExit += OnProcessExit;
120117
}
121118

122-
public void Dispose() => TraceEventSession.Dispose();
123-
124-
internal void Stop()
119+
public override void Dispose()
125120
{
126-
TraceEventSession.Stop();
127-
128-
Console.CancelKeyPress -= OnConsoleCancelKeyPress;
129-
AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
121+
TraceEventSession.Dispose();
122+
base.Dispose();
130123
}
131124

132125
internal abstract Session EnableProviders();
133126

134-
private void OnConsoleCancelKeyPress(object sender, ConsoleCancelEventArgs e) => Stop();
135-
136-
private void OnProcessExit(object sender, EventArgs e) => Stop();
137-
138127
protected static string GetSessionName(BenchmarkCase benchmarkCase)
139128
{
140129
string benchmarkName = FullNameProvider.GetBenchmarkName(benchmarkCase);
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
using System;
2+
3+
namespace BenchmarkDotNet.Helpers
4+
{
5+
/// <summary>
6+
/// Ensures that Dispose is called at termination of the Process.
7+
/// </summary>
8+
public class DisposeAtProcessTermination : IDisposable
9+
{
10+
public DisposeAtProcessTermination()
11+
{
12+
Console.CancelKeyPress += OnCancelKeyPress;
13+
AppDomain.CurrentDomain.ProcessExit += OnProcessExit;
14+
// It does not make sense to include a finalizer. As we are subscribed to static events,
15+
// it will never be called.
16+
}
17+
18+
/// <summary>
19+
/// Called when the user presses Ctrl-C or Ctrl-Break.
20+
/// </summary>
21+
private void OnCancelKeyPress(object? sender, ConsoleCancelEventArgs e)
22+
{
23+
if (!e.Cancel) { Dispose(); }
24+
}
25+
26+
/// <summary>
27+
/// Called when the user clicks on the X in the upper right corner to close the Benchmark's Window.
28+
/// </summary>
29+
private void OnProcessExit(object? sender, EventArgs e) => Dispose();
30+
31+
public virtual void Dispose()
32+
{
33+
Console.CancelKeyPress -= OnCancelKeyPress;
34+
AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
35+
}
36+
}
37+
}

src/BenchmarkDotNet/Helpers/TaskbarProgress.cs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ internal enum TaskbarProgressState
1414
Warning = Paused
1515
}
1616

17-
internal class TaskbarProgress : IDisposable
17+
internal class TaskbarProgress : DisposeAtProcessTermination
1818
{
1919
private static readonly bool OsVersionIsSupported = OsDetector.IsWindows()
2020
// Must be windows 7 or greater
@@ -31,10 +31,6 @@ internal TaskbarProgress(TaskbarProgressState initialTaskbarState)
3131
{
3232
com = Com.MaybeCreateInstanceAndSetInitialState(initialTaskbarState);
3333
terminal = Terminal.MaybeCreateInstanceAndSetInitialState(initialTaskbarState);
34-
if (IsEnabled)
35-
{
36-
Console.CancelKeyPress += OnConsoleCancelEvent;
37-
}
3834
}
3935
}
4036

@@ -51,23 +47,20 @@ internal void SetProgress(float progressValue)
5147
{
5248
throw new ArgumentOutOfRangeException(nameof(progressValue), "progressValue must be between 0 and 1 inclusive.");
5349
}
54-
uint value = (uint) (progressValue * 100);
50+
uint value = (uint)(progressValue * 100);
5551
com?.SetValue(value);
5652
terminal?.SetValue(value);
5753
}
5854

59-
private void OnConsoleCancelEvent(object sender, ConsoleCancelEventArgs e)
60-
=> Dispose();
61-
62-
public void Dispose()
55+
public override void Dispose()
6356
{
6457
if (IsEnabled)
6558
{
6659
SetState(TaskbarProgressState.NoProgress);
6760
com = null;
6861
terminal = null;
69-
Console.CancelKeyPress -= OnConsoleCancelEvent;
7062
}
63+
base.Dispose();
7164
}
7265

7366
private sealed class Com

src/BenchmarkDotNet/Running/ConsoleTitler.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
using System;
22
using System.IO;
33
using BenchmarkDotNet.Detectors;
4-
using BenchmarkDotNet.Portability;
4+
using BenchmarkDotNet.Helpers;
55

66
namespace BenchmarkDotNet.Running
77
{
88
/// <summary>
99
/// Updates Console.Title, subject to platform capabilities and Console availability.
1010
/// Restores the original (or fallback) title upon disposal.
1111
/// </summary>
12-
internal class ConsoleTitler : IDisposable
12+
internal class ConsoleTitler : DisposeAtProcessTermination
1313
{
1414
/// <summary>
1515
/// Whether this instance has any effect. This will be false if the platform doesn't support Console retitling,
@@ -76,13 +76,14 @@ public void UpdateTitle(string title)
7676
}
7777
}
7878

79-
public void Dispose()
79+
public override void Dispose()
8080
{
8181
if (IsEnabled)
8282
{
8383
Console.Title = oldConsoleTitle;
8484
IsEnabled = false;
8585
}
86+
base.Dispose();
8687
}
8788
}
8889
}

src/BenchmarkDotNet/Running/PowerManagementApplier.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
using BenchmarkDotNet.Environments;
55
using BenchmarkDotNet.Helpers;
66
using BenchmarkDotNet.Loggers;
7-
using BenchmarkDotNet.Portability;
87

98
namespace BenchmarkDotNet.Running
109
{
11-
internal class PowerManagementApplier : IDisposable
10+
internal class PowerManagementApplier : DisposeAtProcessTermination
1211
{
1312
private static readonly Guid UserPowerPlan = new Guid("67b4a053-3646-4532-affd-0535c9ea82a7");
1413

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

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

31-
public void Dispose() => ApplyUserPowerPlan();
30+
public override void Dispose()
31+
{
32+
ApplyUserPowerPlan();
33+
base.Dispose();
34+
}
3235

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

0 commit comments

Comments
 (0)