Wrapped concurrentqueue + semaphore pair together for readability, and unblocked recovery main loop during retry delay.

Author: michac

projects/client/RabbitMQ.Client/src/client/impl/AutorecoveringConnection.cs

@@ -47,6 +47,7 @@
 
 using RabbitMQ.Client.Events;
 using RabbitMQ.Client.Impl;
+using RabbitMQ.Util;
 
 namespace RabbitMQ.Client.Framing.Impl
 {
@@ -477,7 +478,6 @@ private void Init(IFrameHandler fh)
                 if (ShouldTriggerConnectionRecovery(args))
                 {
                     _recoveryLoopCommandQueue.Enqueue(RecoveryCommand.BeginAutomaticRecovery);
-                    _semaphore.Release();
                 }
             };
             lock (_eventLock)
@@ -961,8 +961,7 @@ private enum RecoveryConnectionState
         private Task _recoveryTask;
         private RecoveryConnectionState _recoveryLoopState = RecoveryConnectionState.Connected;
 
-        private readonly ConcurrentQueue<RecoveryCommand> _recoveryLoopCommandQueue = new ConcurrentQueue<RecoveryCommand>();
-        private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(0);
+        private readonly AsyncConcurrentQueue<RecoveryCommand> _recoveryLoopCommandQueue = new AsyncConcurrentQueue<RecoveryCommand>();
         private readonly CancellationTokenSource _recoveryCancellationToken = new CancellationTokenSource();
         private readonly TaskCompletionSource<int> _recoveryLoopComplete = new TaskCompletionSource<int>();
 
@@ -975,29 +974,19 @@ private async Task MainRecoveryLoop()
             {
                 while (!_recoveryCancellationToken.IsCancellationRequested)
                 {
-                    try
-                    {
-                        await _semaphore.WaitAsync(_recoveryCancellationToken.Token).ConfigureAwait(false);
-                    }
-                    catch (TaskCanceledException)
-                    {
-                        // Swallowing the task cancellation in case we are stopping work.
-                    }
-
-                    if (!_recoveryCancellationToken.IsCancellationRequested && _recoveryLoopCommandQueue.TryDequeue(out RecoveryCommand command))
+                    var command = await _recoveryLoopCommandQueue.DequeueAsync(_recoveryCancellationToken.Token).ConfigureAwait(false);
+                    
+                    switch (_recoveryLoopState)
                     {
-                        switch (_recoveryLoopState)
-                        {
-                            case RecoveryConnectionState.Connected:
-                                await RecoveryLoopConnectedHandler(command).ConfigureAwait(false);
-                                break;
-                            case RecoveryConnectionState.Recovering:
-                                await RecoveryLoopRecoveringHandler(command).ConfigureAwait(false);
-                                break;
-                            default:
-                                ESLog.Warn("RecoveryLoop state is out of range.");
-                                break;
-                        }
+                        case RecoveryConnectionState.Connected:
+                            RecoveryLoopConnectedHandler(command);
+                            break;
+                        case RecoveryConnectionState.Recovering:
+                            RecoveryLoopRecoveringHandler(command);
+                            break;
+                        default:
+                            ESLog.Warn("RecoveryLoop state is out of range.");
+                            break;
                     }
                 }
             }
@@ -1030,7 +1019,7 @@ private void StopRecoveryLoop()
         /// Handles commands when in the Recovering state.
         /// </summary>
         /// <param name="command"></param>
-        private async Task RecoveryLoopRecoveringHandler(RecoveryCommand command)
+        private void RecoveryLoopRecoveringHandler(RecoveryCommand command)
         {
             switch (command)
             {
@@ -1044,9 +1033,7 @@ private async Task RecoveryLoopRecoveringHandler(RecoveryCommand command)
                     }
                     else
                     {
-                        await Task.Delay(_factory.NetworkRecoveryInterval);
-                        _recoveryLoopCommandQueue.Enqueue(RecoveryCommand.PerformAutomaticRecovery);
-                        _semaphore.Release();
+                        ScheduleRecoveryRetry();
                     }
 
                     break;
@@ -1060,7 +1047,7 @@ private async Task RecoveryLoopRecoveringHandler(RecoveryCommand command)
         /// Handles commands when in the Connected state.
         /// </summary>
         /// <param name="command"></param>
-        private async Task RecoveryLoopConnectedHandler(RecoveryCommand command)
+        private void RecoveryLoopConnectedHandler(RecoveryCommand command)
         {
             switch (command)
             {
@@ -1069,14 +1056,24 @@ private async Task RecoveryLoopConnectedHandler(RecoveryCommand command)
                     break;
                 case RecoveryCommand.BeginAutomaticRecovery:
                     _recoveryLoopState = RecoveryConnectionState.Recovering;
-                    await Task.Delay(_factory.NetworkRecoveryInterval).ConfigureAwait(false);
-                    _recoveryLoopCommandQueue.Enqueue(RecoveryCommand.PerformAutomaticRecovery);
-                    _semaphore.Release();
+                    ScheduleRecoveryRetry();
                     break;
                 default:
                     ESLog.Warn($"RecoveryLoop command {command} is out of range.");
                     break;
             }
         }
+
+        /// <summary>
+        /// Schedule a background Task to signal the command queue when the retry duration has elapsed.
+        /// </summary>
+        private void ScheduleRecoveryRetry()
+        {
+            Task.Delay(_factory.NetworkRecoveryInterval)
+                .ContinueWith(t =>
+                {
+                    _recoveryLoopCommandQueue.Enqueue(RecoveryCommand.PerformAutomaticRecovery);
+                });
+        }
     }
 }

projects/client/RabbitMQ.Client/src/util/AsyncConcurrentQueue.cs

@@ -0,0 +1,86 @@
+// This source code is dual-licensed under the Apache License, version
+// 2.0, and the Mozilla Public License, version 1.1.
+//
+// The APL v2.0:
+//
+//---------------------------------------------------------------------------
+//   Copyright (c) 2007-2020 VMware, Inc.
+//
+//   Licensed under the Apache License, Version 2.0 (the "License");
+//   you may not use this file except in compliance with the License.
+//   You may obtain a copy of the License at
+//
+//       https://www.apache.org/licenses/LICENSE-2.0
+//
+//   Unless required by applicable law or agreed to in writing, software
+//   distributed under the License is distributed on an "AS IS" BASIS,
+//   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//   See the License for the specific language governing permissions and
+//   limitations under the License.
+//---------------------------------------------------------------------------
+//
+// The MPL v1.1:
+//
+//---------------------------------------------------------------------------
+//  The contents of this file are subject to the Mozilla Public License
+//  Version 1.1 (the "License"); you may not use this file except in
+//  compliance with the License. You may obtain a copy of the License
+//  at https://www.mozilla.org/MPL/
+//
+//  Software distributed under the License is distributed on an "AS IS"
+//  basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See
+//  the License for the specific language governing rights and
+//  limitations under the License.
+//
+//  The Original Code is RabbitMQ.
+//
+//  The Initial Developer of the Original Code is Pivotal Software, Inc.
+//  Copyright (c) 2007-2020 VMware, Inc.  All rights reserved.
+//---------------------------------------------------------------------------
+
+using System;
+using System.Collections.Concurrent;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace RabbitMQ.Util
+{
+    /// <summary>
+    /// A concurrent queue where Dequeue waits for something to be inserted if the queue is empty and Enqueue signals
+    ///   that something has been added. Similar in function to a BlockingCollection but with async/await
+    ///   support.
+    /// </summary>
+    /// <typeparam name="T"></typeparam>
+    internal class AsyncConcurrentQueue<T>
+    {
+        private readonly ConcurrentQueue<T> _internalQueue = new ConcurrentQueue<T>();
+        private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(0);
+
+        /// <summary>
+        /// Returns a Task that is completed when an object can be returned from the beginning of the queue.
+        /// </summary>
+        /// <param name="token"></param>
+        /// <returns></returns>
+        public async Task<T> DequeueAsync(CancellationToken token = default)
+        {
+            await _semaphore.WaitAsync(token).ConfigureAwait(false);
+
+            if (!_internalQueue.TryDequeue(out var command))
+            {
+                throw new InvalidOperationException("Internal queue empty despite signaled enqueue.");
+            }
+
+            return command;
+        }
+
+        /// <summary>
+        /// Add an object to the end of the queue.
+        /// </summary>
+        /// <param name="item"></param>
+        public void Enqueue(T item)
+        {
+            _internalQueue.Enqueue(item);
+            _semaphore.Release();
+        }
+    }
+}