From 00a4fae534561c0c978ce7feb3ff94a1a0bfc2f1 Mon Sep 17 00:00:00 2001 From: Christian <6939810+chkr1011@users.noreply.github.com> Date: Wed, 26 Oct 2022 20:00:53 +0200 Subject: [PATCH 1/5] No longer fire event _ClientAcknowledgedPublishPacket_ when a PUBREC packet is received. --- ...lientAcknowledgedPublishPacketEventArgs.cs | 16 ++++++++++---- Source/MQTTnet/Server/Internal/MqttClient.cs | 22 +++++-------------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/Source/MQTTnet/Server/Events/ClientAcknowledgedPublishPacketEventArgs.cs b/Source/MQTTnet/Server/Events/ClientAcknowledgedPublishPacketEventArgs.cs index e0c09ab16..5b126060c 100644 --- a/Source/MQTTnet/Server/Events/ClientAcknowledgedPublishPacketEventArgs.cs +++ b/Source/MQTTnet/Server/Events/ClientAcknowledgedPublishPacketEventArgs.cs @@ -10,15 +10,23 @@ namespace MQTTnet.Server { public sealed class ClientAcknowledgedPublishPacketEventArgs : EventArgs { + public ClientAcknowledgedPublishPacketEventArgs(string clientId, IDictionary sessionItems, MqttPublishPacket publishPacket, MqttPacketWithIdentifier acknowledgePacket) + { + ClientId = clientId ?? throw new ArgumentNullException(nameof(clientId)); + SessionItems = sessionItems ?? throw new ArgumentNullException(nameof(sessionItems)); + PublishPacket = publishPacket ?? throw new ArgumentNullException(nameof(publishPacket)); + AcknowledgePacket = acknowledgePacket ?? throw new ArgumentNullException(nameof(acknowledgePacket)); + } + /// /// Gets the packet which was used for acknowledge. This can be a PubAck or PubComp packet. /// - public MqttPacketWithIdentifier AcknowledgePacket { get; internal set; } + public MqttPacketWithIdentifier AcknowledgePacket { get; } /// /// Gets the ID of the client which acknowledged a PUBLISH packet. /// - public string ClientId { get; internal set; } + public string ClientId { get; } /// /// Gets whether the PUBLISH packet is fully acknowledged. This is the case for PUBACK (QoS 1) and PUBCOMP (QoS 2. @@ -28,11 +36,11 @@ public sealed class ClientAcknowledgedPublishPacketEventArgs : EventArgs /// /// Gets the PUBLISH packet which was acknowledged. /// - public MqttPublishPacket PublishPacket { get; internal set; } + public MqttPublishPacket PublishPacket { get; } /// /// Gets the session items which contain custom user data per session. /// - public IDictionary SessionItems { get; internal set; } + public IDictionary SessionItems { get; } } } \ No newline at end of file diff --git a/Source/MQTTnet/Server/Internal/MqttClient.cs b/Source/MQTTnet/Server/Internal/MqttClient.cs index 4ae2d40d7..94e0e2503 100644 --- a/Source/MQTTnet/Server/Internal/MqttClient.cs +++ b/Source/MQTTnet/Server/Internal/MqttClient.cs @@ -170,14 +170,7 @@ Task ClientAcknowledgedPublishPacket(MqttPublishPacket publishPacket, MqttPacket { if (_eventContainer.ClientAcknowledgedPublishPacketEvent.HasHandlers) { - var eventArgs = new ClientAcknowledgedPublishPacketEventArgs - { - PublishPacket = publishPacket, - AcknowledgePacket = acknowledgePacket, - ClientId = Id, - SessionItems = Session.Items - }; - + var eventArgs = new ClientAcknowledgedPublishPacketEventArgs(Id, Session.Items, publishPacket, acknowledgePacket); return _eventContainer.ClientAcknowledgedPublishPacketEvent.TryInvokeAsync(eventArgs, _logger); } @@ -272,17 +265,14 @@ async Task HandleIncomingPublishPacket(MqttPublishPacket publishPacket, Cancella } } - async Task HandleIncomingPubRecPacket(MqttPubRecPacket pubRecPacket) + Task HandleIncomingPubRecPacket(MqttPubRecPacket pubRecPacket) { - var acknowledgedPublishPacket = Session.PeekAcknowledgePublishPacket(pubRecPacket.PacketIdentifier); - - if (acknowledgedPublishPacket != null) - { - await ClientAcknowledgedPublishPacket(acknowledgedPublishPacket, pubRecPacket).ConfigureAwait(false); - } - + // Do not fire the event _ClientAcknowledgedPublishPacket_ here because the QoS 2 process is only finished + // properly when the client has sent the PUBCOMP packet. var pubRelPacket = _packetFactories.PubRel.Create(pubRecPacket, MqttApplicationMessageReceivedReasonCode.Success); Session.EnqueueControlPacket(new MqttPacketBusItem(pubRelPacket)); + + return CompletedTask.Instance; } void HandleIncomingPubRelPacket(MqttPubRelPacket pubRelPacket) From 5c46b26263b8dc6ba675af119d89bd422aa5d82e Mon Sep 17 00:00:00 2001 From: Christian <6939810+chkr1011@users.noreply.github.com> Date: Wed, 26 Oct 2022 20:01:50 +0200 Subject: [PATCH 2/5] Refactor code. --- MQTTnet.sln.DotSettings | 1 + Source/MQTTnet/Server/Internal/MqttClient.cs | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/MQTTnet.sln.DotSettings b/MQTTnet.sln.DotSettings index 7eed65371..83eed4bd6 100644 --- a/MQTTnet.sln.DotSettings +++ b/MQTTnet.sln.DotSettings @@ -242,5 +242,6 @@ See the LICENSE file in the project root for more information. True True True + True True True \ No newline at end of file diff --git a/Source/MQTTnet/Server/Internal/MqttClient.cs b/Source/MQTTnet/Server/Internal/MqttClient.cs index 94e0e2503..cb003f83e 100644 --- a/Source/MQTTnet/Server/Internal/MqttClient.cs +++ b/Source/MQTTnet/Server/Internal/MqttClient.cs @@ -11,7 +11,6 @@ using MQTTnet.Diagnostics; using MQTTnet.Exceptions; using MQTTnet.Formatter; -using MQTTnet.Implementations; using MQTTnet.Internal; using MQTTnet.PacketDispatcher; using MQTTnet.Packets; From 87a3c08c2aa9911202517f4aa8ef5eaadac27060 Mon Sep 17 00:00:00 2001 From: Christian <6939810+chkr1011@users.noreply.github.com> Date: Wed, 26 Oct 2022 20:21:27 +0200 Subject: [PATCH 3/5] Fix unit tests. --- Source/MQTTnet.Tests/Server/QoS_Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/MQTTnet.Tests/Server/QoS_Tests.cs b/Source/MQTTnet.Tests/Server/QoS_Tests.cs index 08bc6219d..b2a2bfcfe 100644 --- a/Source/MQTTnet.Tests/Server/QoS_Tests.cs +++ b/Source/MQTTnet.Tests/Server/QoS_Tests.cs @@ -99,7 +99,7 @@ public async Task Fire_Event_On_Client_Acknowledges_QoS_2() await LongTestDelay(); - Assert.AreEqual(2, eventArgs.Count); + Assert.AreEqual(1, eventArgs.Count); var firstEvent = eventArgs[0]; From e204d7afef383b45d55d277ab226fabc88a28d85 Mon Sep 17 00:00:00 2001 From: Christian <6939810+chkr1011@users.noreply.github.com> Date: Wed, 26 Oct 2022 20:57:39 +0200 Subject: [PATCH 4/5] Fix unit tests. --- Source/MQTTnet.Tests/Server/QoS_Tests.cs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Source/MQTTnet.Tests/Server/QoS_Tests.cs b/Source/MQTTnet.Tests/Server/QoS_Tests.cs index b2a2bfcfe..7e5a9491a 100644 --- a/Source/MQTTnet.Tests/Server/QoS_Tests.cs +++ b/Source/MQTTnet.Tests/Server/QoS_Tests.cs @@ -106,18 +106,9 @@ public async Task Fire_Event_On_Client_Acknowledges_QoS_2() Assert.IsNotNull(firstEvent); Assert.IsNotNull(firstEvent.PublishPacket); Assert.IsNotNull(firstEvent.AcknowledgePacket); - Assert.IsFalse(firstEvent.IsCompleted); + Assert.IsTrue(firstEvent.IsCompleted); Assert.AreEqual("A", firstEvent.PublishPacket.Topic); - - var secondEvent = eventArgs[1]; - - Assert.IsNotNull(secondEvent); - Assert.IsNotNull(secondEvent.PublishPacket); - Assert.IsNotNull(secondEvent.AcknowledgePacket); - Assert.IsTrue(secondEvent.IsCompleted); - - Assert.AreEqual("A", secondEvent.PublishPacket.Topic); } } } From 80330db9e24d82b50fdd7ea5889a9c37ebc27d41 Mon Sep 17 00:00:00 2001 From: Christian <6939810+chkr1011@users.noreply.github.com> Date: Sun, 6 Nov 2022 10:27:32 +0100 Subject: [PATCH 5/5] Update ReleaseNotes.md --- .github/workflows/ReleaseNotes.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ReleaseNotes.md b/.github/workflows/ReleaseNotes.md index 8dee16631..687239f2b 100644 --- a/.github/workflows/ReleaseNotes.md +++ b/.github/workflows/ReleaseNotes.md @@ -1,2 +1 @@ -* [Core] Fixed dead lock and race conditions in new _AsyncLock_ implementation (#1542). -* [Client] Fixed connection freeze when using Xamarin etc. \ No newline at end of file +* [Server] Fixed duplicated invocation of the event _ClientAcknowledgedPublishPacketAsync_ for QoS level 2 (#1550) \ No newline at end of file