From a848fdfedf6850b3ae024f4042f020f8f349df3a Mon Sep 17 00:00:00 2001 From: Kyriakos Sidiropoulos Date: Mon, 9 Dec 2024 19:40:28 +0100 Subject: [PATCH 1/2] refa (TaskCompletionSourceExtensions.cs): refine the implementation of the TCS extensions to have them handle more elegantly corner-case exceptions --- .../Helpers/TaskCompletionSourceExtensions.cs | 111 ++++++++++++------ .../Shared/DeviceResetter/DeviceResetter.cs | 2 +- .../Shared/FileDownloader/FileDownloader.cs | 2 +- .../Shared/FileUploader/FileUploader.cs | 2 +- .../Shared/FirmwareEraser/FirmwareEraser.cs | 2 +- .../FirmwareInstaller/FirmwareInstaller.cs | 2 +- 6 files changed, 80 insertions(+), 41 deletions(-) diff --git a/Laerdal.McuMgr/Shared/Common/Helpers/TaskCompletionSourceExtensions.cs b/Laerdal.McuMgr/Shared/Common/Helpers/TaskCompletionSourceExtensions.cs index 9fa8c0a..29c4021 100644 --- a/Laerdal.McuMgr/Shared/Common/Helpers/TaskCompletionSourceExtensions.cs +++ b/Laerdal.McuMgr/Shared/Common/Helpers/TaskCompletionSourceExtensions.cs @@ -35,7 +35,7 @@ static internal class TaskCompletionSourceExtensions /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeout); + /// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeout); /// } /// finally /// { @@ -60,11 +60,11 @@ static internal class TaskCompletionSourceExtensions /// } /// } /// - 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); } /// @@ -88,7 +88,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeout); + /// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeout); /// } /// finally /// { @@ -113,11 +113,11 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet /// } /// } /// - 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); } /// @@ -131,7 +131,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet /// /// The task to monitor with a timeout. /// The timeout in milliseconds. If zero or negative it gets interpreted as "wait forever" and the method will - /// just return the task itself. + /// just return the task itself. /// (optional) The cancellation token that co-monitors the waiting mechanism. /// Thrown when the task didn't complete within the specified timeout. /// The hybridized task that you can await on. @@ -142,7 +142,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeout); + /// await tcs.WaitAndFossilizeTaskOnTimeoutAsync(timeout); /// } /// finally /// { @@ -167,9 +167,9 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskComplet /// } /// } /// - 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); } /// @@ -194,7 +194,7 @@ public static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletionSourc /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithTimeoutAsync(timeout); + /// await tcs.WaitAndFossilizeTaskOnTimeoutAsync(timeout); /// } /// finally /// { @@ -219,7 +219,7 @@ public static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletionSourc /// } /// } /// - 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" { @@ -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 @@ -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 } - + /// /// Sets up a timeout-monitor on the given task. This is essentially a wrapper around /// with two major differences:

@@ -279,7 +287,7 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithTimeoutAsync<int>(timeout); + /// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync<int>(timeout); /// } /// finally /// { @@ -304,13 +312,13 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio /// } /// } /// - 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); } - + /// /// Sets up a timeout-monitor on the given task. This is essentially a wrapper around /// with two major differences:

@@ -332,7 +340,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskC /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithOptionalTimeoutAsync<int>(timeout); + /// await tcs.WaitAndFossilizeTaskOnOptionalTimeoutAsync<int>(timeout); /// } /// finally /// { @@ -357,11 +365,11 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskC /// } /// } /// - 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); } /// @@ -385,7 +393,7 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskC /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithTimeoutAsync<int>(timeout); + /// await tcs.WaitAndFossilizeTaskOnTimeoutAsync<int>(timeout); /// } /// finally /// { @@ -410,9 +418,9 @@ public static Task WaitAndFossilizeTaskWithOptionalTimeoutAsync(this TaskC /// } /// } /// - 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); } /// @@ -436,7 +444,7 @@ public static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio /// try /// { /// PropertyChanged += MyEventHandler_; - /// await tcs.WaitAndFossilizeTaskWithTimeoutAsync<int>(timeout); + /// await tcs.WaitAndFossilizeTaskOnTimeoutAsync<int>(timeout); /// } /// finally /// { @@ -461,7 +469,7 @@ public static Task WaitAndFossilizeTaskWithTimeoutAsync(this TaskCompletio /// } /// } /// - 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" { @@ -470,16 +478,24 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(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 @@ -498,5 +514,28 @@ public async static Task WaitAndFossilizeTaskWithTimeoutAsync(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 + } } } \ No newline at end of file diff --git a/Laerdal.McuMgr/Shared/DeviceResetter/DeviceResetter.cs b/Laerdal.McuMgr/Shared/DeviceResetter/DeviceResetter.cs index 97d77a6..4fe5861 100644 --- a/Laerdal.McuMgr/Shared/DeviceResetter/DeviceResetter.cs +++ b/Laerdal.McuMgr/Shared/DeviceResetter/DeviceResetter.cs @@ -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) { diff --git a/Laerdal.McuMgr/Shared/FileDownloader/FileDownloader.cs b/Laerdal.McuMgr/Shared/FileDownloader/FileDownloader.cs index 43b8117..da7805b 100644 --- a/Laerdal.McuMgr/Shared/FileDownloader/FileDownloader.cs +++ b/Laerdal.McuMgr/Shared/FileDownloader/FileDownloader.cs @@ -285,7 +285,7 @@ public async Task DownloadAsync( if (verdict != EFileDownloaderVerdict.Success) throw new ArgumentException(verdict.ToString()); - result = await taskCompletionSource.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeoutForDownloadInMs); + result = await taskCompletionSource.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeoutForDownloadInMs); break; } catch (TimeoutException ex) diff --git a/Laerdal.McuMgr/Shared/FileUploader/FileUploader.cs b/Laerdal.McuMgr/Shared/FileUploader/FileUploader.cs index 2425092..b5a3053 100644 --- a/Laerdal.McuMgr/Shared/FileUploader/FileUploader.cs +++ b/Laerdal.McuMgr/Shared/FileUploader/FileUploader.cs @@ -352,7 +352,7 @@ public async Task UploadAsync( if (verdict != EFileUploaderVerdict.Success) throw new ArgumentException(verdict.ToString()); - await taskCompletionSource.WaitAndFossilizeTaskWithOptionalTimeoutAsync(timeoutForUploadInMs); //order + await taskCompletionSource.WaitAndFossilizeTaskOnOptionalTimeoutAsync(timeoutForUploadInMs); //order break; } catch (TimeoutException ex) diff --git a/Laerdal.McuMgr/Shared/FirmwareEraser/FirmwareEraser.cs b/Laerdal.McuMgr/Shared/FirmwareEraser/FirmwareEraser.cs index 4e66ef5..c64c5da 100644 --- a/Laerdal.McuMgr/Shared/FirmwareEraser/FirmwareEraser.cs +++ b/Laerdal.McuMgr/Shared/FirmwareEraser/FirmwareEraser.cs @@ -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) { diff --git a/Laerdal.McuMgr/Shared/FirmwareInstaller/FirmwareInstaller.cs b/Laerdal.McuMgr/Shared/FirmwareInstaller/FirmwareInstaller.cs index f76bea1..021fbd4 100644 --- a/Laerdal.McuMgr/Shared/FirmwareInstaller/FirmwareInstaller.cs +++ b/Laerdal.McuMgr/Shared/FirmwareInstaller/FirmwareInstaller.cs @@ -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) { From b3622f3647f87eb6a2d0af87f7a5ba917883b9a8 Mon Sep 17 00:00:00 2001 From: Kyriakos Sidiropoulos Date: Mon, 9 Dec 2024 21:59:39 +0100 Subject: [PATCH 2/2] feat (Laerdal.McuMgr.Bindings.Android.NativeBuilder.targets): upgrade libs nordicsemi-android-mcumgr-ble-2.2.2.aar and nordicsemi-android-mcumgr-core-2.2.2.aar we had to upgrade jackson libs as well as Xamarin.Kotlin.StdLib, Xamarin.KotlinX.Coroutines.Android and Xamarin.AndroidX.Core --- .../mcumgr-laerdal-wrapper/build.gradle | 6 +++--- ...Mgr.Bindings.Android.NativeBuilder.targets | 20 +++++++++---------- ...erdal.McuMgr.Bindings.Android.NetX.targets | 4 ++-- .../Laerdal.McuMgr.Bindings.Android.csproj | 5 +++-- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/Laerdal.McuMgr.Bindings.Android.Native/mcumgr-laerdal-wrapper/build.gradle b/Laerdal.McuMgr.Bindings.Android.Native/mcumgr-laerdal-wrapper/build.gradle index f823170..71fceda 100644 --- a/Laerdal.McuMgr.Bindings.Android.Native/mcumgr-laerdal-wrapper/build.gradle +++ b/Laerdal.McuMgr.Bindings.Android.Native/mcumgr-laerdal-wrapper/build.gradle @@ -37,11 +37,11 @@ android { } dependencies { - implementation group: 'com.google.code.gson', name: 'gson', version: '2.10.1' + implementation group: 'com.google.code.gson', name: 'gson', version: '2.11.0' implementation 'androidx.appcompat:appcompat:1.7.0' - implementation 'no.nordicsemi.android:mcumgr-ble:2.2.1' - implementation 'no.nordicsemi.android:mcumgr-core:2.2.1' + implementation 'no.nordicsemi.android:mcumgr-ble:2.2.2' + implementation 'no.nordicsemi.android:mcumgr-core:2.2.2' implementation 'com.google.android.material:material:1.12.0' } diff --git a/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NativeBuilder.targets b/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NativeBuilder.targets index b8eacda..a04a525 100644 --- a/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NativeBuilder.targets +++ b/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NativeBuilder.targets @@ -98,10 +98,10 @@ - - - - + + + + @@ -110,18 +110,18 @@ - - + + - - + + - - + + diff --git a/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NetX.targets b/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NetX.targets index 9896922..b095e32 100644 --- a/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NetX.targets +++ b/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.NetX.targets @@ -38,8 +38,8 @@ - - + + diff --git a/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.csproj b/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.csproj index 1f20100..d7ae76e 100644 --- a/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.csproj +++ b/Laerdal.McuMgr.Bindings.Android/Laerdal.McuMgr.Bindings.Android.csproj @@ -132,11 +132,12 @@ - + - + +