Skip to content

Commit

Permalink
refa (TaskCompletionSourceExtensions.cs): refine the implementation o…
Browse files Browse the repository at this point in the history 
…f the TCS extensions to have them handle more elegantly corner-case exceptions
  • Loading branch information
@ksidirop-laerdal
ksidirop-laerdal committed Dec 9, 2024
1 parent bc783bc commit a848fdf
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 41 deletions.
111 changes: 75 additions & 36 deletions Laerdal.McuMgr/Shared/Common/Helpers/TaskCompletionSourceExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static internal class TaskCompletionSourceExtensions
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeout);
/// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeout);
/// }
/// finally
/// {
Expand All @@ -60,11 +60,11 @@ static internal class TaskCompletionSourceExtensions
/// }
/// }
/// </code>
public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskCompletionSource tcs, int timeoutInMilliseconds, CancellationToken cancellationToken = default)
static public Task WaitAndFossilizeTaskOnOptionalTimeoutAsync(this TaskCompletionSource tcs, long timeoutInMilliseconds, CancellationToken cancellationToken = default)
{
return timeoutInMilliseconds <= 0
? tcs.Task
: tcs.WaitAndFossilizeTaskWithTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
: tcs.WaitAndFossilizeTaskOnTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
}

/// <summary>
Expand All @@ -88,7 +88,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeout);
/// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeout);
/// }
/// finally
/// {
Expand All @@ -113,11 +113,11 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet
/// }
/// }
/// </code>
public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskCompletionSource tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
static public Task WaitAndFossilizeTaskOnOptionalTimeoutAsync(this TaskCompletionSource tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
{
return timespan <= TimeSpan.Zero
? tcs.Task
: tcs.WaitAndFossilizeTaskWithTimeoutAsync(timespan, cancellationToken);
: tcs.WaitAndFossilizeTaskOnTimeoutAsync(timespan, cancellationToken);
}

/// <summary>
Expand All @@ -131,7 +131,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet
/// </summary>
/// <param name="tcs">The task to monitor with a timeout.</param>
/// <param name="timeoutInMilliseconds">The timeout in milliseconds. If zero or negative it gets interpreted as "wait forever" and the method will
/// just return the task itself.</param>
/// just return the task itself.</param>
/// <param name="cancellationToken">(optional) The cancellation token that co-monitors the waiting mechanism.</param>
/// <exception cref="TimeoutException">Thrown when the task didn't complete within the specified timeout.</exception>
/// <returns>The hybridized task that you can await on.</returns>
Expand All @@ -142,7 +142,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeout);
/// await tcs.WaitAndFossilizeTaskOnTimeoutAsync(timeout);
/// }
/// finally
/// {
Expand All @@ -167,9 +167,9 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet
/// }
/// }
/// </code>
public static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletionSource tcs, int timeoutInMilliseconds, CancellationToken cancellationToken = default)
static public Task WaitAndFossilizeTaskOnTimeoutAsync(this TaskCompletionSource tcs, long timeoutInMilliseconds, CancellationToken cancellationToken = default)
{
return tcs.WaitAndFossilizeTaskWithTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
return tcs.WaitAndFossilizeTaskOnTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
}

/// <summary>
Expand All @@ -194,7 +194,7 @@ public static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletionSourc
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithTimeoutAsync(timeout);
/// await tcs.WaitAndFossilizeTaskOnTimeoutAsync(timeout);
/// }
/// finally
/// {
Expand All @@ -219,7 +219,7 @@ public static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletionSourc
/// }
/// }
/// </code>
public async static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletionSource tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
static public async Task WaitAndFossilizeTaskOnTimeoutAsync(this TaskCompletionSource tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
{
if (timespan < TimeSpan.Zero) //note that this deviates from the behaviour of .WaitAsync() which does accept -1 milliseconds which means "wait forever"
{
Expand All @@ -228,17 +228,25 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio

throw exception;
}

try
{
await tcs.Task.WaitAsync(timespan, cancellationToken);
}
catch (Exception ex) when (ex is TimeoutException or TaskCanceledException) //taskcanceledexception can come from cancellation-token timeouts
catch (Exception ex) when (ex is TimeoutException or OperationCanceledException or TaskCanceledException) //taskcanceledexception and operationcanceledexception can come from cancellation-token timeouts
{
var isCancellationSuccessful = tcs.TrySetCanceled(cancellationToken); //00 vital
if (isCancellationSuccessful)
throw;

var properExceptionToThrow = DeduceProperExceptionToThrow(ex, cancellationToken);
var isFossilizationSuccessful = properExceptionToThrow is TimeoutException
? tcs.TrySetException(properExceptionToThrow) //00 vital
: tcs.TrySetCanceled(cancellationToken); //00 vital
if (isFossilizationSuccessful)
{
if (properExceptionToThrow == ex)
throw; //if possible we want to throw the original exception to preserve the original stack-trace

throw properExceptionToThrow;
}

if (tcs.Task.IsCompletedSuccessfully) //10 barely completed in time
return; //micro-optimization to avoid the overhead of await

Expand All @@ -257,7 +265,7 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio
// 2. that the task itself threw a timeout-exception and in this case we prefer to honor the exception that
// the task itself threw and let it be propagated to the caller
}

/// <summary>
/// Sets up a timeout-monitor on the given task. This is essentially a wrapper around <see cref="System.Threading.Tasks.Task.WaitAsync(TimeSpan)"/>
/// with two major differences:<br/><br/>
Expand All @@ -279,7 +287,7 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithTimeoutAsync&lt;int&gt;(timeout);
/// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync&lt;int&gt;(timeout);
/// }
/// finally
/// {
Expand All @@ -304,13 +312,13 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio
/// }
/// }
/// </code>
public static Task<T> WaitAndFossilizeTaskWithOptionalTimeoutAsync<T>(this TaskCompletionSource<T> tcs, int timeoutInMilliseconds, CancellationToken cancellationToken = default)
static public Task<T> WaitAndFossilizeTaskOnOptionalTimeoutAsync<T>(this TaskCompletionSource<T> tcs, long timeoutInMilliseconds, CancellationToken cancellationToken = default)
{
return timeoutInMilliseconds <= 0
? tcs.Task
: tcs.WaitAndFossilizeTaskWithTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
: tcs.WaitAndFossilizeTaskOnTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
}

/// <summary>
/// Sets up a timeout-monitor on the given task. This is essentially a wrapper around <see cref="System.Threading.Tasks.Task.WaitAsync(TimeSpan)"/>
/// with two major differences:<br/><br/>
Expand All @@ -332,7 +340,7 @@ public static Task<T> WaitAndFossilizeTaskWithOptionalTimeoutAsync<T>(this TaskC
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync&lt;int&gt;(timeout);
/// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync&lt;int&gt;(timeout);
/// }
/// finally
/// {
Expand All @@ -357,11 +365,11 @@ public static Task<T> WaitAndFossilizeTaskWithOptionalTimeoutAsync<T>(this TaskC
/// }
/// }
/// </code>
public static Task<T> WaitAndFossilizeTaskWithOptionalTimeoutAsync<T>(this TaskCompletionSource<T> tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
static public Task<T> WaitAndFossilizeTaskOnOptionalTimeoutAsync<T>(this TaskCompletionSource<T> tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
{
return timespan <= TimeSpan.Zero
? tcs.Task
: tcs.WaitAndFossilizeTaskWithTimeoutAsync(timespan, cancellationToken);
: tcs.WaitAndFossilizeTaskOnTimeoutAsync(timespan, cancellationToken);
}

/// <summary>
Expand All @@ -385,7 +393,7 @@ public static Task<T> WaitAndFossilizeTaskWithOptionalTimeoutAsync<T>(this TaskC
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithTimeoutAsync&lt;int&gt;(timeout);
/// await tcs.WaitAndFossilizeTaskOnTimeoutAsync&lt;int&gt;(timeout);
/// }
/// finally
/// {
Expand All @@ -410,9 +418,9 @@ public static Task<T> WaitAndFossilizeTaskWithOptionalTimeoutAsync<T>(this TaskC
/// }
/// }
/// </code>
public static Task<T> WaitAndFossilizeTaskWithTimeoutAsync<T>(this TaskCompletionSource<T> tcs, int timeoutInMilliseconds, CancellationToken cancellationToken = default)
static public Task<T> WaitAndFossilizeTaskOnTimeoutAsync<T>(this TaskCompletionSource<T> tcs, long timeoutInMilliseconds, CancellationToken cancellationToken = default)
{
return tcs.WaitAndFossilizeTaskWithTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
return tcs.WaitAndFossilizeTaskOnTimeoutAsync(TimeSpan.FromMilliseconds(timeoutInMilliseconds), cancellationToken);
}

/// <summary>
Expand All @@ -436,7 +444,7 @@ public static Task<T> WaitAndFossilizeTaskWithTimeoutAsync<T>(this TaskCompletio
/// try
/// {
/// PropertyChanged += MyEventHandler_;
/// await tcs.WaitAndFossilizeTaskWithTimeoutAsync&lt;int&gt;(timeout);
/// await tcs.WaitAndFossilizeTaskOnTimeoutAsync&lt;int&gt;(timeout);
/// }
/// finally
/// {
Expand All @@ -461,7 +469,7 @@ public static Task<T> WaitAndFossilizeTaskWithTimeoutAsync<T>(this TaskCompletio
/// }
/// }
/// </code>
public async static Task<T> WaitAndFossilizeTaskWithTimeoutAsync<T>(this TaskCompletionSource<T> tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
static public async Task<T> WaitAndFossilizeTaskOnTimeoutAsync<T>(this TaskCompletionSource<T> tcs, TimeSpan timespan, CancellationToken cancellationToken = default)
{
if (timespan < TimeSpan.Zero) //note that this deviates from the behaviour of .WaitAsync() which does accept -1 milliseconds which means "wait forever"
{
Expand All @@ -470,16 +478,24 @@ public async static Task<T> WaitAndFossilizeTaskWithTimeoutAsync<T>(this TaskCom

throw exception;
}

try
{
return await tcs.Task.WaitAsync(timespan, cancellationToken);
}
catch (Exception ex) when (ex is TimeoutException or TaskCanceledException) //taskcanceledexception can come from cancellation-token timeouts
catch (Exception ex) when (ex is TimeoutException or OperationCanceledException or TaskCanceledException) //taskcanceledexception and operationcanceledexception can come from cancellation-token timeouts
{
var isCancellationSuccessful = tcs.TrySetCanceled(cancellationToken); //00 vital
if (isCancellationSuccessful)
throw;
var properExceptionToThrow = DeduceProperExceptionToThrow(ex, cancellationToken);
var isFossilizationSuccessful = properExceptionToThrow is TimeoutException
? tcs.TrySetException(properExceptionToThrow) //00 vital
: tcs.TrySetCanceled(cancellationToken); //00 vital
if (isFossilizationSuccessful)
{
if (properExceptionToThrow == ex)
throw; //if possible we want to throw the original exception to preserve the original stack-trace

throw properExceptionToThrow;
}

return tcs.Task.IsCompletedSuccessfully //10 barely completed in time
? tcs.Task.Result //micro-optimization to avoid the overhead of await
Expand All @@ -498,5 +514,28 @@ public async static Task<T> WaitAndFossilizeTaskWithTimeoutAsync<T>(this TaskCom
// 2. that the task itself threw a timeout-exception and in this case we prefer to honor the exception that
// the task itself threw and let it be propagated to the caller
}

static private Exception DeduceProperExceptionToThrow(Exception ex, CancellationToken cancellationToken)
{
var exceptionToThrow = ex switch
{
TimeoutException => ex,

OperationCanceledException ocex => //00
cancellationToken.IsCancellationRequested //10 known small bug here
? ex
: new TimeoutException(message: "Operation timed out", innerException: ocex),

_ => ex,
};

return exceptionToThrow;

//00 bear in mind that TaskCanceledException derives from OperationCanceledException
//
//10 todo: unfortunately the cancellation token does not allow us to differentiate between a timeout and a cancellation and we thus have a small bug here due
// todo: to this: we throw an OperationCanceledException in the case of a timeout instead of a TimeoutException there is a feature request about this
// https://github.com/dotnet/runtime/discussions/110461 and https://github.com/dotnet/runtime/issues/109246
}
}
}
2 changes: 1 addition & 1 deletion Laerdal.McuMgr/Shared/DeviceResetter/DeviceResetter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public async Task ResetAsync(int timeoutInMs = -1)
if (verdict != EDeviceResetterInitializationVerdict.Success)
throw new ArgumentException(verdict.ToString());

await taskCompletionSource.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeoutInMs);
await taskCompletionSource.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeoutInMs);
}
catch (TimeoutException ex)
{
Expand Down
2 changes: 1 addition & 1 deletion Laerdal.McuMgr/Shared/FileDownloader/FileDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public async Task<byte[]> DownloadAsync(
if (verdict != EFileDownloaderVerdict.Success)
throw new ArgumentException(verdict.ToString());

result = await taskCompletionSource.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeoutForDownloadInMs);
result = await taskCompletionSource.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeoutForDownloadInMs);
break;
}
catch (TimeoutException ex)
Expand Down
2 changes: 1 addition & 1 deletion Laerdal.McuMgr/Shared/FileUploader/FileUploader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public async Task UploadAsync<TData>(
if (verdict != EFileUploaderVerdict.Success)
throw new ArgumentException(verdict.ToString());

await taskCompletionSource.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeoutForUploadInMs); //order
await taskCompletionSource.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeoutForUploadInMs); //order
break;
}
catch (TimeoutException ex)
Expand Down
2 changes: 1 addition & 1 deletion Laerdal.McuMgr/Shared/FirmwareEraser/FirmwareEraser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public async Task EraseAsync(int imageIndex = 1, int timeoutInMs = -1)
if (verdict != EFirmwareErasureInitializationVerdict.Success)
throw new ArgumentException(verdict.ToString());

await taskCompletionSource.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeoutInMs);
await taskCompletionSource.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeoutInMs);
}
catch (TimeoutException ex)
{
Expand Down
2 changes: 1 addition & 1 deletion Laerdal.McuMgr/Shared/FirmwareInstaller/FirmwareInstaller.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public async Task InstallAsync(
if (verdict != EFirmwareInstallationVerdict.Success)
throw new ArgumentException(verdict.ToString());

await taskCompletionSource.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeoutInMs);
await taskCompletionSource.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeoutInMs);
}
catch (TimeoutException ex)
{
Expand Down

0 comments on commit a848fdf

Please sign in to comment.