diff --git a/NModbus.UnitTests/Device/ModbusTcpSlaveNetworkFixture.cs b/NModbus.UnitTests/Device/ModbusTcpSlaveNetworkFixture.cs
new file mode 100644
index 0000000..d5afbab
--- /dev/null
+++ b/NModbus.UnitTests/Device/ModbusTcpSlaveNetworkFixture.cs
@@ -0,0 +1,153 @@
+using System;
+using System.Collections;
+using System.Net;
+using System.Net.Sockets;
+using System.Reflection;
+using System.Threading;
+using System.Threading.Tasks;
+using NModbus.Device;
+using NModbus.Logging;
+using Xunit;
+
+namespace NModbus.UnitTests.Device
+{
+ ///
+ /// Regression coverage for issue #135: when a master TCP connection closed,
+ /// ModbusTcpSlaveNetwork.OnMasterConnectionClosedHandler tore the
+ /// connection out of the masters dictionary and disposed it, but never
+ /// unsubscribed its own delegate from the connection's
+ /// ModbusMasterTcpConnectionClosed event. The handler also threw
+ /// from inside an event invocation if the
+ /// endpoint had already been removed (e.g. by a concurrent dispose path),
+ /// which is unsafe behavior for an event sink.
+ ///
+ public class ModbusTcpSlaveNetworkFixture
+ {
+ private static IModbusFactory Factory => new ModbusFactory();
+
+ [Fact]
+ public void OnMasterConnectionClosedHandler_UnknownEndPoint_DoesNotThrow()
+ {
+ using (var listener = new ListenerHandle())
+ using (var network = new ModbusTcpSlaveNetwork(listener.Listener, Factory, NullModbusLogger.Instance))
+ {
+ // Invoke the private handler with an endpoint that was never registered.
+ // Before the fix this threw ArgumentException; after the fix it logs and returns.
+ MethodInfo handler = typeof(ModbusTcpSlaveNetwork).GetMethod(
+ "OnMasterConnectionClosedHandler",
+ BindingFlags.Instance | BindingFlags.NonPublic);
+ Assert.NotNull(handler);
+
+ object eventArgs = new TcpConnectionEventArgs("127.0.0.1:0");
+
+ Exception captured = null;
+ try
+ {
+ handler.Invoke(network, new[] { (object)null, eventArgs });
+ }
+ catch (TargetInvocationException tie)
+ {
+ captured = tie.InnerException;
+ }
+
+ Assert.Null(captured);
+ }
+ }
+
+ [Fact]
+ public async Task ClosingClientConnection_DetachesEventAndRemovesFromMasters()
+ {
+ using (var listener = new ListenerHandle())
+ using (var network = new ModbusTcpSlaveNetwork(listener.Listener, Factory, NullModbusLogger.Instance))
+ using (var cts = new CancellationTokenSource())
+ {
+ Task listenTask = network.ListenAsync(cts.Token);
+
+ using (var client = new TcpClient())
+ {
+ await client.ConnectAsync(IPAddress.Loopback, listener.Port).ConfigureAwait(false);
+
+ // Wait until the slave network registers the connection.
+ IDictionary masters = await WaitForMasterCountAsync(network, expected: 1).ConfigureAwait(false);
+ Assert.Single(masters);
+
+ object connection = FirstValue(masters);
+ EventHandler beforeSubscribers = GetClosedEventSubscribers(connection);
+ Assert.NotNull(beforeSubscribers);
+ Assert.Single(beforeSubscribers.GetInvocationList());
+
+ // Closing the client triggers HandleRequestAsync's 0-byte path which fires the event.
+ client.Close();
+
+ masters = await WaitForMasterCountAsync(network, expected: 0).ConfigureAwait(false);
+ Assert.Empty(masters);
+
+ EventHandler afterSubscribers = GetClosedEventSubscribers(connection);
+ Assert.Null(afterSubscribers);
+ }
+
+ cts.Cancel();
+ try { await listenTask.ConfigureAwait(false); }
+ catch (OperationCanceledException) { /* expected */ }
+ catch (SocketException) { /* AcceptTcpClientAsync may surface this when the listener is stopped */ }
+ }
+ }
+
+ private static async Task WaitForMasterCountAsync(ModbusTcpSlaveNetwork network, int expected)
+ {
+ FieldInfo field = typeof(ModbusTcpSlaveNetwork).GetField(
+ "_masters",
+ BindingFlags.Instance | BindingFlags.NonPublic);
+ var masters = (IDictionary)field.GetValue(network);
+
+ for (int i = 0; i < 50; i++)
+ {
+ if (masters.Count == expected)
+ {
+ return masters;
+ }
+
+ await Task.Delay(100).ConfigureAwait(false);
+ }
+
+ return masters;
+ }
+
+ private static object FirstValue(IDictionary masters)
+ {
+ foreach (object value in masters.Values)
+ {
+ return value;
+ }
+
+ return null;
+ }
+
+ private static EventHandler GetClosedEventSubscribers(object connection)
+ {
+ FieldInfo field = connection.GetType().GetField(
+ "ModbusMasterTcpConnectionClosed",
+ BindingFlags.Instance | BindingFlags.NonPublic);
+ return (EventHandler)field.GetValue(connection);
+ }
+
+ private sealed class ListenerHandle : IDisposable
+ {
+ public ListenerHandle()
+ {
+ Listener = new TcpListener(IPAddress.Loopback, 0);
+ Listener.Start();
+ Port = ((IPEndPoint)Listener.LocalEndpoint).Port;
+ }
+
+ public TcpListener Listener { get; }
+
+ public int Port { get; }
+
+ public void Dispose()
+ {
+ try { Listener.Stop(); } catch { /* ignore */ }
+ }
+ }
+ }
+}
diff --git a/NModbus/Device/ModbusTcpSlaveNetwork.cs b/NModbus/Device/ModbusTcpSlaveNetwork.cs
index 7e7c221..81128cb 100644
--- a/NModbus/Device/ModbusTcpSlaveNetwork.cs
+++ b/NModbus/Device/ModbusTcpSlaveNetwork.cs
@@ -214,12 +214,19 @@ private void OnTimer(object sender, ElapsedEventArgs e)
#endif
private void OnMasterConnectionClosedHandler(object sender, TcpConnectionEventArgs e)
{
- ModbusMasterTcpConnection connection;
+ // Issue #135: detach our handler from the connection so the network
+ // doesn't keep getting called back after the connection is gone, and
+ // so that a re-entrant or double-fired close event can't crash an
+ // event-source thread with ArgumentException.
+ if (sender is ModbusMasterTcpConnection senderConnection)
+ {
+ senderConnection.ModbusMasterTcpConnectionClosed -= OnMasterConnectionClosedHandler;
+ }
- if (!_masters.TryRemove(e.EndPoint, out connection))
+ if (!_masters.TryRemove(e.EndPoint, out ModbusMasterTcpConnection connection))
{
- string msg = $"EndPoint {e.EndPoint} cannot be removed, it does not exist.";
- throw new ArgumentException(msg);
+ Logger.Warning($"EndPoint {e.EndPoint} cannot be removed, it does not exist.");
+ return;
}
connection.Dispose();