Bugfix - Ensure client side timeouts are honored (#6920)

Fixing bug when timeout helper was not initialized correctly and hence client side timeouts were not behaving as expected. Without the `true` argument, the timer doesn't start. For example in `RetryPolicy` which is used everywhere, the timer doesn't start till the first operation is completed. That results in unexpected behavior, and the operations being called for the second time though the timer has expired.
Azure · Jul 17, 2019 · 765ff59 · 765ff59
1 parent 2e5a5e9
commit 765ff59
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 9 deletions.
diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpLinkCreator.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Amqp/AmqpLinkCreator.cs
@@ -35,7 +35,7 @@ protected AmqpLinkCreator(string entityPath, ServiceBusConnection serviceBusConn
 
         public async Task<Tuple<AmqpObject, DateTime>> CreateAndOpenAmqpLinkAsync()
         {
-            var timeoutHelper = new TimeoutHelper(this.serviceBusConnection.OperationTimeout);
+            var timeoutHelper = new TimeoutHelper(this.serviceBusConnection.OperationTimeout, true);
 
             MessagingEventSource.Log.AmqpGetOrCreateConnectionStart();
             var amqpConnection = await this.serviceBusConnection.ConnectionManager.GetOrCreateAsync(timeoutHelper.RemainingTime()).ConfigureAwait(false);

diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/TimeoutHelper.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/Primitives/TimeoutHelper.cs
@@ -13,11 +13,6 @@ struct TimeoutHelper
         bool deadlineSet;
         TimeSpan originalTimeout;
 
-        public TimeoutHelper(TimeSpan timeout)
-            : this(timeout, false)
-        {
-        }
-
         public TimeoutHelper(TimeSpan timeout, bool startTimeout)
         {
             Debug.Assert(timeout >= TimeSpan.Zero, "timeout must be non-negative");

diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/RetryPolicy.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/RetryPolicy.cs
@@ -60,7 +60,7 @@ public async Task RunOperation(Func<Task> operation, TimeSpan operationTimeout)
         {
             var currentRetryCount = 0;
             List<Exception> exceptions = null;
-            var timeoutHelper = new TimeoutHelper(operationTimeout);
+            var timeoutHelper = new TimeoutHelper(operationTimeout, true);
 
             if (this.IsServerBusy && timeoutHelper.RemainingTime() < RetryPolicy.ServerBusyBaseSleepTime)
             {

diff --git a/sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnection.cs b/sdk/servicebus/Microsoft.Azure.ServiceBus/src/ServiceBusConnection.cs
@@ -244,7 +244,7 @@ async Task<AmqpConnection> CreateConnectionAsync(TimeSpan timeout)
         {
             var hostName = this.Endpoint.Host;
 
-            var timeoutHelper = new TimeoutHelper(timeout);
+            var timeoutHelper = new TimeoutHelper(timeout, true);
             var amqpSettings = AmqpConnectionHelper.CreateAmqpSettings(
                 amqpVersion: AmqpVersion,
                 useSslStreamSecurity: true,
@@ -274,7 +274,7 @@ async Task<AmqpConnection> CreateConnectionAsync(TimeSpan timeout)
 
         async Task<Controller> CreateControllerAsync(TimeSpan timeout)
         {
-            var timeoutHelper = new TimeoutHelper(timeout);
+            var timeoutHelper = new TimeoutHelper(timeout, true);
             var connection = await this.ConnectionManager.GetOrCreateAsync(timeoutHelper.RemainingTime()).ConfigureAwait(false);
 
             var sessionSettings = new AmqpSessionSettings { Properties = new Fields() };