Azure / azure-service-bus-dotnet

☁️ .NET Standard client library for Azure Service Bus
https://azure.microsoft.com/services/service-bus
Other
235 stars 120 forks source link

ActiveClientLinkManager throws ArgumentOutOfRangeException in SetRenewCbsTokenTimer. #672

Open zlatanov opened 5 years ago

zlatanov commented 5 years ago

Actual Behavior

ArgumentOutOfRangeException is thrown. Number must be either non-negative and less than or equal to Int32.MaxValue or -1. Parameter name: dueTime.

Call Stack:

Microsoft.Azure.ServiceBus.Amqp.ActiveClientLinkManager.ChangeRenewTimer(Microsoft.Azure.ServiceBus.Amqp.ActiveClientLinkObject activeClientLinkObject, System.TimeSpan dueTime) Line 149
Microsoft.Azure.ServiceBus.Amqp.ActiveClientLinkManager.SetRenewCbsTokenTimer(Microsoft.Azure.ServiceBus.Amqp.ActiveClientLinkObject activeClientLinkObject) Line 137
Microsoft.Azure.ServiceBus.Amqp.ActiveClientLinkManager.RenewCbsTokenAsync(Microsoft.Azure.ServiceBus.Amqp.ActiveClientLinkObject activeClientLinkObject) Line 111

https://github.com/Azure/azure-service-bus-dotnet/blob/e1f6e531ac8dc2417d59c03a983d9987e3850989/src/Microsoft.Azure.ServiceBus/Amqp/ActiveClientLinkManager.cs#L126-L136

The interval here becomes negative. ActiveClientLinkManager.TokenRefreshBuffer is 10 seconds according to the debugger. Because the refresh buffer is subtracted this makes line 128 incorrect - it doesn't take it into consideration.

Also I think that even if it wasn't subtracted, the code would still have issues because the DateTime.UtcNow is called twice, which could return different times which in turn makes the check on line 128 obsolete.

Versions

axisc commented 5 years ago

@zlatanov - thanks for reporting this, we'll investigate and fix internally.