Skip to content

Commit cb732f9

Browse files
Switch timer type to one that throws fewer exceptions
1 parent 5536593 commit cb732f9

File tree

3 files changed

+19
-23
lines changed

3 files changed

+19
-23
lines changed

CodeConverter/Shared/OptionalOperations.cs

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Threading;
3-
using System.Timers;
43
using Microsoft.CodeAnalysis;
54
using Microsoft.CodeAnalysis.Formatting;
65

@@ -22,7 +21,9 @@ public OptionalOperations(TimeSpan abandonTasksIfNoActivityFor, IProgress<Conver
2221
_progress = progress;
2322
_wholeTaskCancellationToken = wholeTaskCancellationToken;
2423
_optionalTaskCts = CancellationTokenSource.CreateLinkedTokenSource(wholeTaskCancellationToken);
25-
_activityMonitor = new ActivityMonitor(abandonTasksIfNoActivityFor, _optionalTaskCts);
24+
_activityMonitor = new ActivityMonitor(abandonTasksIfNoActivityFor, _optionalTaskCts, () =>
25+
_progress.Report(new ConversionProgress("WARNING: Skipping all further formatting, you can increase the timeout for this in Tools -> Options -> Code Converter."))
26+
);
2627
}
2728

2829
public SyntaxNode MapSourceTriviaToTargetHandled<TSource, TTarget>(TSource root,
@@ -52,14 +53,11 @@ public SyntaxNode Format(SyntaxNode node, Document document)
5253
cancellationToken: _optionalTaskCts.Token);
5354

5455
} catch (OperationCanceledException) {
55-
if (!_wholeTaskCancellationToken.IsCancellationRequested) {
56-
_progress.Report(new ConversionProgress(
57-
"Aborting all further formatting and comment mapping, you can increase the timeout for this in Tools -> Options -> Code Converter."));
58-
}
5956
} finally {
6057
_activityMonitor.ActivityFinished();
6158
}
6259
}
60+
_progress.Report(new ConversionProgress("Skipped formatting", 1));
6361
return node.NormalizeWhitespace();
6462
}
6563

@@ -70,36 +68,40 @@ private class ActivityMonitor
7068
{
7169
private readonly TimeSpan _timeout;
7270
private readonly CancellationTokenSource _cts;
71+
private readonly Action _afterInactivity;
7372
private volatile int _activeOperations;
7473
/// <summary>
7574
/// Must check <see cref="_activeOperations"/> within the lock before changed timer.Enabled
7675
/// This avoids race conditions between the last task of a set finishing and the first of a new set starting
7776
/// </summary>
7877
private readonly object _timerEnabledWriteLock = new object();
79-
private static System.Timers.Timer _timer;
78+
private static Timer _timer;
8079

8180

82-
private void OnTimedEvent(object source, ElapsedEventArgs e)
81+
private void OnTimedEvent(object state)
8382
{
84-
if (!_cts.IsCancellationRequested && _timer.Enabled) {
83+
if (!_cts.IsCancellationRequested && _activeOperations > 0) {
8584
_cts.Cancel();
85+
_afterInactivity();
86+
} else if (_activeOperations > 0) {
87+
ActivityObserved();
8688
}
8789
}
8890

89-
public ActivityMonitor(TimeSpan timeout, CancellationTokenSource cts)
91+
public ActivityMonitor(TimeSpan timeout, CancellationTokenSource cts, Action afterInactivity)
9092
{
9193
_timeout = timeout;
9294
_cts = cts;
93-
_timer = new System.Timers.Timer(timeout.TotalMilliseconds) {AutoReset = true};
94-
_timer.Elapsed += OnTimedEvent;
95+
_afterInactivity = afterInactivity;
96+
_timer = new Timer(OnTimedEvent, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
9597
}
9698

9799
public void ActivityStarted()
98100
{
99101
if (Interlocked.Increment(ref _activeOperations) == 1) {
100102
lock (_timerEnabledWriteLock) {
101103
if (_activeOperations > 0) {
102-
_timer.Enabled = true;
104+
ActivityObserved();
103105
}
104106
}
105107
}
@@ -112,21 +114,15 @@ public void ActivityFinished()
112114
if (Interlocked.Decrement(ref _activeOperations) == 0) {
113115
lock (_timerEnabledWriteLock) {
114116
if (_activeOperations == 0) {
115-
_timer.Enabled = false;
117+
_timer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
116118
}
117119
}
118120
}
119121
}
120122

121123
private void ActivityObserved()
122124
{
123-
try {
124-
_timer.Interval = _timeout.TotalMilliseconds;
125-
} catch (ObjectDisposedException e) {
126-
// Race condition if we try to set the interval after disabling the timer
127-
} catch (NullReferenceException e) {
128-
// Race condition if we try to set the interval after disabling the timer
129-
}
125+
_timer.Change(_timeout, Timeout.InfiniteTimeSpan);
130126
}
131127
}
132128
}

CommandLine/CodeConv.Shared/MSBuildWorkspaceConverter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public async IAsyncEnumerable<ConversionResult> ConvertProjectsWhereAsync(Func<P
5757
var languageConversion = targetLanguage == Language.CS
5858
? (ILanguageConversion)new VBToCSConversion()
5959
: new CSToVBConversion();
60-
languageConversion.ConversionOptions.AbandonOptionalTasksAfter = TimeSpan.FromHours(4);
60+
languageConversion.ConversionOptions = new ConversionOptions(){AbandonOptionalTasksAfter = TimeSpan.FromHours(4)};
6161
var languageNameToConvert = targetLanguage == Language.CS
6262
? LanguageNames.VisualBasic
6363
: LanguageNames.CSharp;

Vsix/ConverterOptionsPage.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ internal sealed class ConverterOptionsPage : DialogPage
2424

2525
[Category(SettingsPageCategory)]
2626
[DisplayName("Comment and formatting timeout (minutes)")]
27-
[Description("Positioning comments correctly, and formatting and tidying up the result can take a very long time for large files. Set this to how many minutes you're willing to wait.")]
27+
[Description("Roslyn formatting can take a very long time for large files and has no progress updates. Set this to the maximum you're willing to wait without any indication of progress.")]
2828
public int FormattingTimeout{ get; set; } = 15;
2929
}
3030

0 commit comments

Comments
 (0)