dotnet / MQTTnet

MQTTnet is a high performance .NET library for MQTT based communication. It provides a MQTT client and a MQTT server (broker). The implementation is based on the documentation from http://mqtt.org/.
MIT License
4.51k stars 1.07k forks source link

Issue using MqttClientConnectionContextFactory because of bug in MqttConnectionContext #1818

Closed CZEMacLeod closed 1 year ago

CZEMacLeod commented 1 year ago

Describe the bug

When creating a client connection using MqttClientConnectionContextFactory in an AspNetCore project, an error is thrown and the client does not connect. MqttClientConnectionContextFactory passes a TcpConnection object to MqttConnectionContext for the ConnectionContext parameter in the constructor. The constructor tries to store the connection.Transport.Input and connection.Transport.Output into local fields, however connection.Transport is null because StartAsync on TcpConnection has not yet been called. https://github.com/dotnet/MQTTnet/blob/b1dc8607af12c2ec6e2e27ab843f315676c82681/Source/MQTTnet.AspnetCore/MqttConnectionContext.cs#L36-L37 There is code in ConnectAsync that is meant to address this, but the code never gets there since the constructor throws an NRE. https://github.com/dotnet/MQTTnet/blob/b1dc8607af12c2ec6e2e27ab843f315676c82681/Source/MQTTnet.AspnetCore/MqttConnectionContext.cs#L109-L118

Which component is your bug related to?

To Reproduce

Steps to reproduce the behavior:

  1. Using this version of MQTTnet 4.2.1.781
  2. Run this code run the MQTTnet.Benchmarks project and select the MessageProcessingMqttConnectionContextBenchmark

Expected behavior

It should not throw and error. The transport properties should be initialized in only one place under ConnectAsync.

MQTTnet - Benchmarks (net6.0)
-----------------------------------------------
Press arrow keys for benchmark selection
Press Enter for benchmark execution
Press Esc for exit

   AsyncLockBenchmark
   ChannelAdapterBenchmark
   LoggerBenchmark
   MessageDeliveryBenchmark
   MessageProcessingBenchmark
-> MessageProcessingMqttConnectionContextBenchmark
   MqttPacketReaderWriterBenchmark
   MqttTcpChannelBenchmark
   RoundtripProcessingBenchmark
   SendPacketAsyncBenchmark
   SerializerBenchmark
   ServerProcessingBenchmark
   SubscribeBenchmark
   TcpPipesBenchmark
   TopicFilterComparerBenchmark
   UnsubscribeBenchmark

// Validating benchmarks:
// ***** BenchmarkRunner: Start   *****
// ***** Found 1 benchmark(s) in total *****
// ***** Building 1 exe(s) in Parallel: Start   *****
// start dotnet  restore /p:UseSharedCompilation=false /p:BuildInParallel=false /m:1 /p:Deterministic=true /p:Optimize=true in S:\source\github\dotnet\MQTTnet\Source\MQTTnet.Benchmarks\bin\Release\net6.0\edd57436-339a-44aa-9f85-546ad9bd073d
// command took 4.75s and exited with 0
// start dotnet  build -c Release --no-restore /p:UseSharedCompilation=false /p:BuildInParallel=false /m:1 /p:Deterministic=true /p:Optimize=true in S:\source\github\dotnet\MQTTnet\Source\MQTTnet.Benchmarks\bin\Release\net6.0\edd57436-339a-44aa-9f85-546ad9bd073d
// command took 25.56s and exited with 1
// start dotnet  build -c Release --no-restore --no-dependencies /p:UseSharedCompilation=false /p:BuildInParallel=false /m:1 /p:Deterministic=true /p:Optimize=true in S:\source\github\dotnet\MQTTnet\Source\MQTTnet.Benchmarks\bin\Release\net6.0\edd57436-339a-44aa-9f85-546ad9bd073d
// command took 4.59s and exited with 0
// ***** Done, took 00:00:38 (38.47 sec)   *****
// Found 1 benchmarks:
//   MessageProcessingMqttConnectionContextBenchmark.Send_10000_Messages: .NET 6.0(Runtime=.NET 6.0)

Setup power plan (GUID: 8c5e7fda-e8bf-4a96-9a85-a6e23a8c635c FriendlyName: High performance)
// **************************
// Benchmark: MessageProcessingMqttConnectionContextBenchmark.Send_10000_Messages: .NET 6.0(Runtime=.NET 6.0)
// *** Execute ***
// Launch: 1 / 1
// Execute: dotnet edd57436-339a-44aa-9f85-546ad9bd073d.dll --benchmarkName MQTTnet.Benchmarks.MessageProcessingMqttConnectionContextBenchmark.Send_10000_Messages --job ".NET 6.0" --benchmarkId 0 in S:\source\github\dotnet\MQTTnet\Source\MQTTnet.Benchmarks\bin\Release\net6.0\edd57436-339a-44aa-9f85-546ad9bd073d\bin\Release\net6.0
// BeforeAnythingElse

// Benchmark Process Environment Information:
// Runtime=.NET 6.0.21 (6.0.2123.36311), X64 RyuJIT AVX2
// GC=Concurrent Workstation
// HardwareIntrinsics=AVX2,AES,BMI1,BMI2,FMA,LZCNT,PCLMUL,POPCNT VectorSize=256
// Job: .NET 6.0

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at MQTTnet.AspNetCore.MqttConnectionContext..ctor(MqttPacketFormatterAdapter packetFormatterAdapter, ConnectionContext connection) in /_/Source/MQTTnet.AspnetCore/MqttConnectionContext.cs:line 36
   at MQTTnet.AspNetCore.Client.MqttClientConnectionContextFactory.CreateClientAdapter(MqttClientOptions options, MqttPacketInspector packetInspector, IMqttNetLogger logger) in /_/Source/MQTTnet.AspnetCore/Client/MqttClientConnectionContextFactory.cs:line 29
   at MQTTnet.Client.MqttClient.ConnectAsync(MqttClientOptions options, CancellationToken cancellationToken) in /_/Source/MQTTnet/Client/MqttClient.cs:line 123
   at MQTTnet.Client.MqttClient.ConnectAsync(MqttClientOptions options, CancellationToken cancellationToken) in /_/Source/MQTTnet/Client/MqttClient.cs:line 181
   at MQTTnet.Benchmarks.MessageProcessingMqttConnectionContextBenchmark.Setup() in S:\source\github\dotnet\MQTTnet\Source\MQTTnet.Benchmarks\MessageProcessingMqttConnectionContextBenchmark.cs:line 49
   at BenchmarkDotNet.Engines.EngineFactory.CreateReadyToRun(EngineParameters engineParameters)
   at BenchmarkDotNet.Autogenerated.Runnable_0.Run(IHost host, String benchmarkName) in S:\source\github\dotnet\MQTTnet\Source\MQTTnet.Benchmarks\bin\Release\net6.0\edd57436-339a-44aa-9f85-546ad9bd073d\edd57436-339a-44aa-9f85-546ad9bd073d.notcs:line 175
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at BenchmarkDotNet.Autogenerated.UniqueProgramName.AfterAssemblyLoadingAttached(String[] args) in S:\source\github\dotnet\MQTTnet\Source\MQTTnet.Benchmarks\bin\Release\net6.0\edd57436-339a-44aa-9f85-546ad9bd073d\edd57436-339a-44aa-9f85-546ad9bd073d.notcs:line 58
// AfterAll
No Workload Results were obtained from the run.
// Benchmark Process 43792 has exited with code -1.

// ** Remained 0 (0.0%) benchmark(s) to run. Estimated finish 2023-08-19 2:21 (0h 0m from now) **
Successfully reverted power plan (GUID: 8c5e7fda-e8bf-4a96-9a85-a6e23a8c635c FriendlyName: High performance)
// ***** BenchmarkRunner: Finish  *****

// * Export *
  BenchmarkDotNet.Artifacts\results\MQTTnet.Benchmarks.MessageProcessingMqttConnectionContextBenchmark-report.csv
  BenchmarkDotNet.Artifacts\results\MQTTnet.Benchmarks.MessageProcessingMqttConnectionContextBenchmark-report-github.md
  BenchmarkDotNet.Artifacts\results\MQTTnet.Benchmarks.MessageProcessingMqttConnectionContextBenchmark-report.html

// * Detailed results *
MessageProcessingMqttConnectionContextBenchmark.Send_10000_Messages: .NET 6.0(Runtime=.NET 6.0)
Runtime = .NET 6.0.21 (6.0.2123.36311), X64 RyuJIT AVX2; GC = Concurrent Workstation
There are not any results runs

// * Summary *

BenchmarkDotNet=v0.13.2, OS=Windows 11 (10.0.25926.1000)
Intel Core i9-9900KF CPU 3.60GHz (Coffee Lake), 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.100-preview.7.23376.3
  [Host]   : .NET 6.0.21 (6.0.2123.36311), X64 RyuJIT AVX2  [AttachedDebugger]
  .NET 6.0 : .NET 6.0.21 (6.0.2123.36311), X64 RyuJIT AVX2

Job=.NET 6.0  Runtime=.NET 6.0

|              Method | Mean | Error |
|-------------------- |-----:|------:|
| Send_10000_Messages |   NA |    NA |

Benchmarks with issues:
  MessageProcessingMqttConnectionContextBenchmark.Send_10000_Messages: .NET 6.0(Runtime=.NET 6.0)

// * Warnings *
Environment
  Summary -> Benchmark was executed with attached debugger
  Summary -> Detected error exit code from one of the benchmarks. It might be caused by following antivirus software:
        - Windows Defender (windowsdefender://)
Use InProcessEmitToolchain or InProcessNoEmitToolchain to avoid new process creation.

// * Legends *
  Mean  : Arithmetic mean of all measurements
  Error : Half of 99.9% confidence interval
  1 ns  : 1 Nanosecond (0.000000001 sec)

// * Diagnostic Output - MemoryDiagnoser *

// ***** BenchmarkRunner: End *****
Run time: 00:00:00 (0.65 sec), executed benchmarks: 1

Global total time: 00:00:43 (43.7 sec), executed benchmarks: 1
// * Artifacts cleanup *
Press any key to exit
chkr1011 commented 1 year ago

The PR for this issue is merged.