Merge pull request #709 from PaulFake/patch-1

Fixed storage queue race condition

.github/ISSUE_TEMPLATE/bug_report.md

@@ -0,0 +1,43 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: ''
+labels: ''
+assignees: ''
+
+---
+
+### Describe the bug
+A clear and concise description of what the bug is.
+
+### Which project is your bug related to?
+- [x] Client
+- [ ] ManagedClient
+- [ ] MQTTnet.Server standalone
+- [ ] Server
+- [ ] Generic
+
+### To Reproduce
+Steps to reproduce the behavior:
+1. Using this version of MQTTnet '...'.
+2. Run this code '....'.
+3. With these arguments '....'.
+4. See error.
+
+### Expected behavior
+A clear and concise description of what you expected to happen.
+
+### Screenshots
+If applicable, add screenshots to help explain your problem.
+
+### Additional context / logging
+Add any other context about the problem here.
+Include debugging or logging information here:
+
+```batch
+```
+### Code example
+ Please provide full code examples below where possible to make it easier for the developers to check your issues.
+ 
+```csharp
+```

.github/ISSUE_TEMPLATE/custom.md

@@ -0,0 +1,18 @@
+---
+name: Custom issue template
+about: Describe this issue template's purpose here.
+title: ''
+labels: ''
+assignees: ''
+
+---
+
+### Describe your question
+A clear and concise description of what you want to know.
+
+### Which project is your bug related to?
+- [x] Client
+- [ ] ManagedClient
+- [ ] MQTTnet.Server standalone
+- [ ] Server
+- [ ] Generic

.github/ISSUE_TEMPLATE/feature_request.md

@@ -0,0 +1,26 @@
+---
+name: Feature request
+about: Suggest an idea for this project
+title: ''
+labels: ''
+assignees: ''
+
+---
+
+Is your feature request related to a problem? Please describe.
+A clear and concise description of what the problem is. Example. I'm am trying to do [...] but [...]
+
+Which project is your feature request related to?
+ Client
+ ManagedClient
+ MQTTnet.Server standalone
+ Server
+ Generic
+Describe the solution you'd like
+A clear and concise description of what you want to happen.
+
+Describe alternatives you've considered
+A clear and concise description of any alternative solutions or features you've considered.
+
+Additional context
+Add any other context or screenshots about the feature request here.

Source/MQTTnet.Extensions.ManagedClient/ManagedMqttClient.cs

@@ -24,6 +24,8 @@ namespace MQTTnet.Extensions.ManagedClient
 
         private readonly IMqttClient _mqttClient;
         private readonly IMqttNetChildLogger _logger;
+        
+        private readonly AsyncLock _messageQueueLock = new AsyncLock();
 
         private CancellationTokenSource _connectionCancellationToken;
         private CancellationTokenSource _publishingCancellationToken;
@@ -147,7 +149,7 @@ namespace MQTTnet.Extensions.ManagedClient
 
             try
             {
-                lock (_messageQueue)
+                using (await _messageQueueLock.WaitAsync(CancellationToken.None).ConfigureAwait(false))
                 {
                     if (_messageQueue.Count >= Options.MaxPendingMessages)
                     {
@@ -167,6 +169,16 @@ namespace MQTTnet.Extensions.ManagedClient
                     }
 
                     _messageQueue.Enqueue(applicationMessage);
+                    
+                    if (_storageManager != null)
+                    {
+                        if (removedMessage != null)
+                        {
+                            await _storageManager.RemoveAsync(removedMessage).ConfigureAwait(false);
+                        }
+
+                        await _storageManager.AddAsync(applicationMessage).ConfigureAwait(false);
+                    }
                 }
             }
             finally
@@ -181,16 +193,6 @@ namespace MQTTnet.Extensions.ManagedClient
                 }
 
             }
-            
-            if (_storageManager != null)
-            {
-                if (removedMessage != null)
-                {
-                    await _storageManager.RemoveAsync(removedMessage).ConfigureAwait(false);
-                }
-
-                await _storageManager.AddAsync(applicationMessage).ConfigureAwait(false);
-            }
         }
 
         public Task SubscribeAsync(IEnumerable<TopicFilter> topicFilters)
@@ -377,7 +379,7 @@ namespace MQTTnet.Extensions.ManagedClient
             {
                 await _mqttClient.PublishAsync(message.ApplicationMessage).ConfigureAwait(false);
 
-                lock (_messageQueue) //lock to avoid conflict with this.PublishAsync
+                using (await _messageQueueLock.WaitAsync(CancellationToken.None).ConfigureAwait(false)) //lock to avoid conflict with this.PublishAsync
                 {
                     // While publishing this message, this.PublishAsync could have booted this
                     // message off the queue to make room for another (when using a cap
@@ -386,11 +388,11 @@ namespace MQTTnet.Extensions.ManagedClient
                     // it from the queue.  If not, that means this.PublishAsync has already
                     // removed it, in which case we don't want to do anything.
                     _messageQueue.RemoveFirst(i => i.Id.Equals(message.Id));
-                }
-
-                if (_storageManager != null)
-                {
-                    await _storageManager.RemoveAsync(message).ConfigureAwait(false);
+                    
+                    if (_storageManager != null)
+                    {
+                        await _storageManager.RemoveAsync(message).ConfigureAwait(false);
+                    }
                 }
             }
             catch (MqttCommunicationException exception)
@@ -408,14 +410,14 @@ namespace MQTTnet.Extensions.ManagedClient
                     //contradict the expected behavior of QoS 1 and 2, that's also true
                     //for the usage of a message queue cap, so it's still consistent
                     //with prior behavior in that way.
-                    lock (_messageQueue) //lock to avoid conflict with this.PublishAsync
+                    using (await _messageQueueLock.WaitAsync(CancellationToken.None).ConfigureAwait(false)) //lock to avoid conflict with this.PublishAsync
                     {
                         _messageQueue.RemoveFirst(i => i.Id.Equals(message.Id));
-                    }
-
-                    if (_storageManager != null)
-                    {
-                        await _storageManager.RemoveAsync(message).ConfigureAwait(false);
+                        
+                        if (_storageManager != null)
+                        {
+                            await _storageManager.RemoveAsync(message).ConfigureAwait(false);
+                        }
                     }
                 }
             }
@@ -533,4 +535,4 @@ namespace MQTTnet.Extensions.ManagedClient
             _connectionCancellationToken = null;
         }
     }
-}
+}