Skip to content

Commit

Permalink
in dispose only dispose, wrap release in try-catch
Browse files Browse the repository at this point in the history
  • Loading branch information
abhipsaMisra committed Feb 28, 2023
1 parent 7ac512a commit 6da391a
Showing 1 changed file with 134 additions and 83 deletions.
217 changes: 134 additions & 83 deletions iothub/device/src/Pipeline/RetryDelegatingHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ internal class RetryDelegatingHandler : DefaultDelegatingHandler
private bool _eventsEnabled;
private bool _deviceReceiveMessageEnabled;
private bool _isAnEdgeModule = true;
private bool _isDisposing = false;

private Task _transportClosedTask;
private readonly CancellationTokenSource _handleDisconnectCts = new CancellationTokenSource();
Expand Down Expand Up @@ -246,7 +247,16 @@ await _internalRetryPolicy
}
finally
{
_cloudToDeviceMessageSubscriptionSemaphore?.Release();
try
{
_cloudToDeviceMessageSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing cloud-to-device message subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -287,7 +297,16 @@ await _internalRetryPolicy
}
finally
{
_cloudToDeviceMessageSubscriptionSemaphore?.Release();
try
{
_cloudToDeviceMessageSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing cloud-to-device message subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -326,7 +345,16 @@ await _internalRetryPolicy
}
finally
{
_cloudToDeviceMessageSubscriptionSemaphore?.Release();
try
{
_cloudToDeviceMessageSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing cloud-to-device message subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -362,7 +390,16 @@ await _internalRetryPolicy
}
finally
{
_directMethodSubscriptionSemaphore?.Release();
try
{
_directMethodSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing direct method subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -398,7 +435,16 @@ await _internalRetryPolicy
}
finally
{
_directMethodSubscriptionSemaphore?.Release();
try
{
_directMethodSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing direct method subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -435,7 +481,16 @@ await _internalRetryPolicy
}
finally
{
_cloudToDeviceEventSubscriptionSemaphore?.Release();
try
{
_cloudToDeviceEventSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing cloud-to-device event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -472,7 +527,16 @@ await _internalRetryPolicy
}
finally
{
_cloudToDeviceEventSubscriptionSemaphore?.Release();
try
{
_cloudToDeviceEventSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing cloud-to-device event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -508,7 +572,16 @@ await _internalRetryPolicy
}
finally
{
_twinEventsSubscriptionSemaphore?.Release();
try
{
_twinEventsSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing twin event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -544,7 +617,16 @@ await _internalRetryPolicy
}
finally
{
_twinEventsSubscriptionSemaphore?.Release();
try
{
_twinEventsSubscriptionSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing twin event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
},
operationCts.Token)
Expand Down Expand Up @@ -714,7 +796,16 @@ public override async Task CloseAsync(CancellationToken cancellationToken)
if (Logging.IsEnabled)
Logging.Exit(this, cancellationToken, nameof(CloseAsync));

_clientOpenCloseSemaphore?.Release();
try
{
_clientOpenCloseSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing twin event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
Dispose(true);
}
}
Expand Down Expand Up @@ -776,7 +867,16 @@ private async Task EnsureOpenedAsync(bool withRetry, CancellationToken cancellat
}
finally
{
_clientOpenCloseSemaphore?.Release();
try
{
_clientOpenCloseSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing twin event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
}

Expand Down Expand Up @@ -831,7 +931,16 @@ private async Task EnsureOpenedAsync(bool withRetry, TimeoutHelper timeoutHelper
}
finally
{
_clientOpenCloseSemaphore?.Release();
try
{
_clientOpenCloseSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing twin event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
}

Expand Down Expand Up @@ -1064,7 +1173,16 @@ await _internalRetryPolicy.RunWithRetryAsync(async () =>
}
finally
{
_clientOpenCloseSemaphore?.Release();
try
{
_clientOpenCloseSemaphore?.Release();
}
catch (ObjectDisposedException) when (_isDisposing)
{
if (Logging.IsEnabled)
Logging.Error(this, "Tried releasing twin event subscription semaphore but it has already been disposed by client disposal on a separate thread." +
"Ignoring this exception and continuing with client cleanup.");
}
}
}

Expand Down Expand Up @@ -1123,6 +1241,8 @@ protected override void Dispose(bool disposing)

if (!_isDisposed)
{
_isDisposing = true;

base.Dispose(disposing);
if (disposing)
{
Expand All @@ -1132,83 +1252,14 @@ protected override void Dispose(bool disposing)
_cancelPendingOperations?.Cancel();
_cancelPendingOperations?.Dispose();

if (_clientOpenCloseSemaphore != null && _clientOpenCloseSemaphore.CurrentCount == 0)
{
try
{
_clientOpenCloseSemaphore.Release();
}
catch (SemaphoreFullException)
{
if (Logging.IsEnabled)
Logging.Error(this, $"Tried releasing the close-open semaphore before disposing it but its count was already at maximum." +
$" Will ignore the exception and dispose the semaphore.");
}
}
_clientOpenCloseSemaphore?.Dispose();

if (_cloudToDeviceMessageSubscriptionSemaphore != null && _cloudToDeviceMessageSubscriptionSemaphore.CurrentCount == 0)
{
try
{
_cloudToDeviceMessageSubscriptionSemaphore.Release();
}
catch (SemaphoreFullException)
{
if (Logging.IsEnabled)
Logging.Error(this, $"Tried releasing the cloud-to-device message subscription semaphore before disposing it but its count was already at maximum." +
$" Will ignore the exception and dispose the semaphore.");
}
}
_cloudToDeviceMessageSubscriptionSemaphore?.Dispose();

if (_cloudToDeviceEventSubscriptionSemaphore != null && _cloudToDeviceEventSubscriptionSemaphore.CurrentCount == 0)
{
try
{
_cloudToDeviceEventSubscriptionSemaphore.Release();
}
catch (SemaphoreFullException)
{
if (Logging.IsEnabled)
Logging.Error(this, $"Tried releasing the cloud-to-device event subscription sepmaphore before disposing it but its count was already at maximum." +
$" Will ignore the exception and dispose the semaphore.");
}
}
_cloudToDeviceEventSubscriptionSemaphore?.Dispose();

if (_directMethodSubscriptionSemaphore != null && _directMethodSubscriptionSemaphore.CurrentCount == 0)
{
try
{
_directMethodSubscriptionSemaphore.Release();
}
catch (SemaphoreFullException)
{
if (Logging.IsEnabled)
Logging.Error(this, $"Tried releasing the direct method subscription semaphore before disposing it but its count was already at maximum." +
$" Will ignore the exception and dispose the semaphore.");
}
}
_directMethodSubscriptionSemaphore?.Dispose();

if (_twinEventsSubscriptionSemaphore != null && _twinEventsSubscriptionSemaphore.CurrentCount == 0)
{
try
{
_twinEventsSubscriptionSemaphore.Release();
}
catch (SemaphoreFullException)
{
if (Logging.IsEnabled)
Logging.Error(this, $"Tried releasing the twin events subscription semaphore before disposing it but its count was already at maximum." +
$" Will ignore the exception and dispose the semaphore.");
}
}
_twinEventsSubscriptionSemaphore?.Dispose();
}

// the _disposed flag is inherited from the base class DefaultDelegatingHandler and is finally set to null there.
// the _disposed flag is inherited from the base class DefaultDelegatingHandler and is finally set to true there.
}
}
finally
Expand Down

0 comments on commit 6da391a

Please sign in to comment.