Remove not used timeout for receiving.

This commit is contained in:
Christian Kratky
2020-11-02 21:45:23 +01:00
parent 44b7133a1c
commit fb2be79673
15 changed files with 49 additions and 42 deletions
@@ -92,7 +92,7 @@ namespace MQTTnet.AspNetCore
return Task.CompletedTask; return Task.CompletedTask;
} }
public async Task<MqttBasePacket> ReceivePacketAsync(TimeSpan timeout, CancellationToken cancellationToken) public async Task<MqttBasePacket> ReceivePacketAsync(CancellationToken cancellationToken)
{ {
var input = Connection.Transport.Input; var input = Connection.Transport.Input;
@@ -29,7 +29,7 @@ namespace MQTTnet.Adapter
Task SendPacketAsync(MqttBasePacket packet, TimeSpan timeout, CancellationToken cancellationToken); Task SendPacketAsync(MqttBasePacket packet, TimeSpan timeout, CancellationToken cancellationToken);
Task<MqttBasePacket> ReceivePacketAsync(TimeSpan timeout, CancellationToken cancellationToken); Task<MqttBasePacket> ReceivePacketAsync(CancellationToken cancellationToken);
void ResetStatistics(); void ResetStatistics();
} }
+7 -16
View File
@@ -155,42 +155,33 @@ namespace MQTTnet.Adapter
} }
} }
public async Task<MqttBasePacket> ReceivePacketAsync(TimeSpan timeout, CancellationToken cancellationToken) public async Task<MqttBasePacket> ReceivePacketAsync(CancellationToken cancellationToken)
{ {
cancellationToken.ThrowIfCancellationRequested(); cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed(); ThrowIfDisposed();
try try
{ {
ReceivedMqttPacket receivedMqttPacket; var receivedPacket = await ReceiveAsync(cancellationToken).ConfigureAwait(false);
if (timeout == TimeSpan.Zero) if (receivedPacket == null || cancellationToken.IsCancellationRequested)
{
receivedMqttPacket = await ReceiveAsync(cancellationToken).ConfigureAwait(false);
}
else
{
receivedMqttPacket = await MqttTaskTimeout.WaitAsync(ReceiveAsync, timeout, cancellationToken).ConfigureAwait(false);
}
if (receivedMqttPacket == null || cancellationToken.IsCancellationRequested)
{ {
return null; return null;
} }
Interlocked.Add(ref _bytesSent, receivedMqttPacket.TotalLength); Interlocked.Add(ref _bytesSent, receivedPacket.TotalLength);
if (PacketFormatterAdapter.ProtocolVersion == MqttProtocolVersion.Unknown) if (PacketFormatterAdapter.ProtocolVersion == MqttProtocolVersion.Unknown)
{ {
PacketFormatterAdapter.DetectProtocolVersion(receivedMqttPacket); PacketFormatterAdapter.DetectProtocolVersion(receivedPacket);
} }
var packet = PacketFormatterAdapter.Decode(receivedMqttPacket); var packet = PacketFormatterAdapter.Decode(receivedPacket);
if (packet == null) if (packet == null)
{ {
throw new MqttProtocolViolationException("Received malformed packet."); throw new MqttProtocolViolationException("Received malformed packet.");
} }
_logger.Verbose("RX ({0} bytes) <<< {1}", receivedMqttPacket.TotalLength, packet); _logger.Verbose("RX ({0} bytes) <<< {1}", receivedPacket.TotalLength, packet);
return packet; return packet;
} }
+1 -1
View File
@@ -493,7 +493,7 @@ namespace MQTTnet.Client
while (!cancellationToken.IsCancellationRequested) while (!cancellationToken.IsCancellationRequested)
{ {
var packet = await _adapter.ReceivePacketAsync(TimeSpan.Zero, cancellationToken).ConfigureAwait(false); var packet = await _adapter.ReceivePacketAsync(cancellationToken).ConfigureAwait(false);
if (cancellationToken.IsCancellationRequested) if (cancellationToken.IsCancellationRequested)
{ {
@@ -18,10 +18,9 @@ namespace MQTTnet.LowLevelClient
public LowLevelMqttClient(IMqttClientAdapterFactory clientAdapterFactory, IMqttNetLogger logger) public LowLevelMqttClient(IMqttClientAdapterFactory clientAdapterFactory, IMqttNetLogger logger)
{ {
if (clientAdapterFactory is null) throw new ArgumentNullException(nameof(clientAdapterFactory)); _clientAdapterFactory = clientAdapterFactory ?? throw new ArgumentNullException(nameof(clientAdapterFactory));
if (logger is null) throw new ArgumentNullException(nameof(logger));
_clientAdapterFactory = clientAdapterFactory; if (logger is null) throw new ArgumentNullException(nameof(logger));
_logger = logger.CreateScopedLogger(nameof(LowLevelMqttClient)); _logger = logger.CreateScopedLogger(nameof(LowLevelMqttClient));
} }
@@ -95,7 +94,7 @@ namespace MQTTnet.LowLevelClient
try try
{ {
return await _adapter.ReceivePacketAsync(_options.CommunicationTimeout, cancellationToken).ConfigureAwait(false); return await _adapter.ReceivePacketAsync(cancellationToken).ConfigureAwait(false);
} }
catch (Exception) catch (Exception)
{ {
@@ -167,7 +167,7 @@ namespace MQTTnet.Server
{ {
Status = MqttClientConnectionStatus.Running; Status = MqttClientConnectionStatus.Running;
var packet = await _channelAdapter.ReceivePacketAsync(TimeSpan.Zero, cancellationToken).ConfigureAwait(false); var packet = await _channelAdapter.ReceivePacketAsync(cancellationToken).ConfigureAwait(false);
if (packet == null) if (packet == null)
{ {
// The client has closed the connection gracefully. // The client has closed the connection gracefully.
@@ -12,6 +12,7 @@ using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Threading; using System.Threading;
using System.Threading.Tasks; using System.Threading.Tasks;
using OperationCanceledException = System.OperationCanceledException;
namespace MQTTnet.Server namespace MQTTnet.Server
{ {
@@ -59,14 +60,24 @@ namespace MQTTnet.Server
MqttConnectPacket connectPacket; MqttConnectPacket connectPacket;
try try
{ {
var firstPacket = await channelAdapter.ReceivePacketAsync(_options.DefaultCommunicationTimeout, cancellationToken).ConfigureAwait(false); using (var timeoutToken = new CancellationTokenSource(_options.DefaultCommunicationTimeout))
connectPacket = firstPacket as MqttConnectPacket;
if (connectPacket == null)
{ {
_logger.Warning(null, "The first packet from client '{0}' was no 'CONNECT' packet [MQTT-3.1.0-1].", channelAdapter.Endpoint); var firstPacket = await channelAdapter.ReceivePacketAsync(timeoutToken.Token).ConfigureAwait(false);
return; connectPacket = firstPacket as MqttConnectPacket;
if (connectPacket == null)
{
_logger.Warning(null,
"The first packet from client '{0}' was no 'CONNECT' packet [MQTT-3.1.0-1].",
channelAdapter.Endpoint);
return;
}
} }
} }
catch (OperationCanceledException)
{
_logger.Warning(null, "Client '{0}' connected but did not sent a CONNECT packet.", channelAdapter.Endpoint);
return;
}
catch (MqttCommunicationTimedOutException) catch (MqttCommunicationTimedOutException)
{ {
_logger.Warning(null, "Client '{0}' connected but did not sent a CONNECT packet.", channelAdapter.Endpoint); _logger.Warning(null, "Client '{0}' connected but did not sent a CONNECT packet.", channelAdapter.Endpoint);
@@ -105,7 +105,10 @@ namespace MQTTnet.Server
// Execute the disconnection in background so that the keep alive monitor can continue // Execute the disconnection in background so that the keep alive monitor can continue
// with checking other connections. // with checking other connections.
Task.Run(() => connection.StopAsync(MqttDisconnectReasonCode.KeepAliveTimeout)); // We do not need to wait for the task so no await is needed.
// Also the internal state of the connection must be swapped to "Finalizing" because the
// next iteration of the keep alive timer happens.
var _ = connection.StopAsync(MqttDisconnectReasonCode.KeepAliveTimeout);
} }
catch (Exception exception) catch (Exception exception)
{ {
@@ -34,7 +34,7 @@ namespace MQTTnet.AspNetCore.Tests
pipe.Receive.Writer.Complete(); pipe.Receive.Writer.Complete();
await Assert.ThrowsExceptionAsync<MqttCommunicationException>(() => ctx.ReceivePacketAsync(TimeSpan.Zero, CancellationToken.None)); await Assert.ThrowsExceptionAsync<MqttCommunicationException>(() => ctx.ReceivePacketAsync(CancellationToken.None));
} }
[TestMethod] [TestMethod]
@@ -49,10 +49,10 @@ namespace MQTTnet.AspNetCore.Tests
await pipe.Receive.Writer.WriteAsync(writer.AddMqttHeader(MqttControlPacketType.Connect, new byte[0])); await pipe.Receive.Writer.WriteAsync(writer.AddMqttHeader(MqttControlPacketType.Connect, new byte[0]));
await Assert.ThrowsExceptionAsync<MqttProtocolViolationException>(() => ctx.ReceivePacketAsync(TimeSpan.Zero, CancellationToken.None)); await Assert.ThrowsExceptionAsync<MqttProtocolViolationException>(() => ctx.ReceivePacketAsync(CancellationToken.None));
// the first exception should complete the pipes so if someone tries to use the connection after that it should throw immidiatly // the first exception should complete the pipes so if someone tries to use the connection after that it should throw immidiatly
await Assert.ThrowsExceptionAsync<InvalidOperationException>(() => ctx.ReceivePacketAsync(TimeSpan.Zero, CancellationToken.None)); await Assert.ThrowsExceptionAsync<InvalidOperationException>(() => ctx.ReceivePacketAsync(CancellationToken.None));
} }
// COMMENTED OUT DUE TO DEAD LOCK? OR VERY VERY SLOW PERFORMANCE ON LOCAL DEV MACHINE. TEST WAS STILL RUNNING AFTER SEVERAL MINUTES! // COMMENTED OUT DUE TO DEAD LOCK? OR VERY VERY SLOW PERFORMANCE ON LOCAL DEV MACHINE. TEST WAS STILL RUNNING AFTER SEVERAL MINUTES!
@@ -53,7 +53,7 @@ namespace MQTTnet.Benchmarks
for (var i = 0; i < 10000; i++) for (var i = 0; i < 10000; i++)
{ {
_channelAdapter.ReceivePacketAsync(TimeSpan.Zero, CancellationToken.None).GetAwaiter().GetResult(); _channelAdapter.ReceivePacketAsync(CancellationToken.None).GetAwaiter().GetResult();
} }
_stream.Position = 0; _stream.Position = 0;
@@ -169,6 +169,7 @@ namespace MQTTnet.Tests.Mockups
{ {
foreach (var mqttClient in _clients) foreach (var mqttClient in _clients)
{ {
mqttClient.DisconnectAsync().GetAwaiter().GetResult();
mqttClient?.Dispose(); mqttClient?.Dispose();
} }
@@ -51,7 +51,7 @@ namespace MQTTnet.Tests.Mockups
return Task.FromResult(0); return Task.FromResult(0);
} }
public Task<MqttBasePacket> ReceivePacketAsync(TimeSpan timeout, CancellationToken cancellationToken) public Task<MqttBasePacket> ReceivePacketAsync(CancellationToken cancellationToken)
{ {
ThrowIfPartnerIsNull(); ThrowIfPartnerIsNull();
@@ -161,6 +161,8 @@ namespace MQTTnet.Tests
SpinWait.SpinUntil(() => replyReceived, TimeSpan.FromSeconds(10)); SpinWait.SpinUntil(() => replyReceived, TimeSpan.FromSeconds(10));
await Task.Delay(500);
Assert.IsTrue(replyReceived); Assert.IsTrue(replyReceived);
} }
} }
@@ -21,6 +21,7 @@ namespace MQTTnet.Tests
var client = await testEnvironment.ConnectLowLevelClientAsync(o => o var client = await testEnvironment.ConnectLowLevelClientAsync(o => o
.WithCommunicationTimeout(TimeSpan.FromSeconds(1)) .WithCommunicationTimeout(TimeSpan.FromSeconds(1))
.WithCommunicationTimeout(TimeSpan.Zero)
.WithProtocolVersion(MqttProtocolVersion.V500)).ConfigureAwait(false); .WithProtocolVersion(MqttProtocolVersion.V500)).ConfigureAwait(false);
await client.SendAsync(new MqttConnectPacket await client.SendAsync(new MqttConnectPacket
@@ -35,17 +36,18 @@ namespace MQTTnet.Tests
for (var i = 0; i < 6; i++) for (var i = 0; i < 6; i++)
{ {
await client.SendAsync(MqttPingReqPacket.Instance, CancellationToken.None);
await Task.Delay(500); await Task.Delay(500);
await client.SendAsync(MqttPingReqPacket.Instance, CancellationToken.None);
responsePacket = await client.ReceiveAsync(CancellationToken.None); responsePacket = await client.ReceiveAsync(CancellationToken.None);
Assert.IsTrue(responsePacket is MqttPingRespPacket); Assert.IsTrue(responsePacket is MqttPingRespPacket);
} }
// If we reach this point everything works as expected (server did not close the connection // If we reach this point everything works as expected (server did not close the connection
// due to proper ping messages. // due to proper ping messages.
// Now we will wait 1.2 seconds because the server MUST wait 1.5 seconds in total (See spec). // Now we will wait 1.1 seconds because the server MUST wait 1.5 seconds in total (See spec).
await Task.Delay(1200); await Task.Delay(1100);
await client.SendAsync(MqttPingReqPacket.Instance, CancellationToken.None); await client.SendAsync(MqttPingReqPacket.Instance, CancellationToken.None);
responsePacket = await client.ReceiveAsync(CancellationToken.None); responsePacket = await client.ReceiveAsync(CancellationToken.None);
Assert.IsTrue(responsePacket is MqttPingRespPacket); Assert.IsTrue(responsePacket is MqttPingRespPacket);
+2 -4
View File
@@ -96,9 +96,7 @@ namespace MQTTnet.Tests
var serverOptions = new MqttServerOptionsBuilder(); var serverOptions = new MqttServerOptionsBuilder();
await testEnvironment.StartServerAsync(serverOptions); await testEnvironment.StartServerAsync(serverOptions);
var options = new MqttClientOptionsBuilder() var options = new MqttClientOptionsBuilder().WithClientId("1");
.WithClientId("1")
;
var clients = await Task.WhenAll(Enumerable.Range(0, 10) var clients = await Task.WhenAll(Enumerable.Range(0, 10)
.Select(i => TryConnect(testEnvironment, options))); .Select(i => TryConnect(testEnvironment, options)));
@@ -109,7 +107,7 @@ namespace MQTTnet.Tests
} }
} }
private async Task<IMqttClient> TryConnect(TestEnvironment testEnvironment, MqttClientOptionsBuilder options) async Task<IMqttClient> TryConnect(TestEnvironment testEnvironment, MqttClientOptionsBuilder options)
{ {
try try
{ {