diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs index c937c893..9996bc1d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs @@ -255,6 +255,30 @@ public sealed class ModbusDriver WriteHealth(new DriverHealth(DriverState.Initializing, null, null)); try { + // Release whatever a PREVIOUS initialize published before building the replacements. All + // three fields assigned below (_transport, _probeCts, _reprobeCts) were simply overwritten, + // so a second initialize on a live driver stranded a connected transport plus two running + // background loops that no later dispose could reach. Reachable without any teardown of + // ours: DriverInstanceActor re-runs InitializeAsync with no ShutdownAsync when an ApplyDelta + // lands while the driver is in Connecting/Reconnecting (AdoptConfigDuringInit). + // Deliberately NOT the shared TeardownAsync(): that also disposes _poll, the PollGroupEngine + // constructed once in the ctor and never rebuilt, which would leave the re-initialized + // driver unable to serve subscriptions. A first initialize sees three nulls, so this is a + // no-op on the initial call. + try { _probeCts?.Cancel(); } catch { } + _probeCts?.Dispose(); + _probeCts = null; + + try { _reprobeCts?.Cancel(); } catch { } + _reprobeCts?.Dispose(); + _reprobeCts = null; + + if (_transport is not null) + { + await _transport.DisposeAsync().ConfigureAwait(false); + _transport = null; + } + // #516: re-parse the supplied DriverConfig so a config change delivered through the IDriver // contract is honoured. Without this the driver keeps serving the options it was CONSTRUCTED // with, so an operator's edit is discarded while the deployment still seals green. An empty / diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs index c82b234e..b67adce0 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs @@ -218,6 +218,93 @@ public sealed class ModbusLifecycleHygieneTests "DisposeAsync must dispose the PollGroupEngine so its background Task stops, not just the transport"); } + // -------------------- Re-initialize transport hygiene -------------------- + + /// + /// A second InitializeAsync on a live driver — the shape + /// DriverInstanceActor.AdoptConfigDuringInit produces when an ApplyDelta lands while the + /// driver is Connecting/Reconnecting — must dispose the transport the previous initialize + /// published before publishing a new one. Pre-fix _transport was simply overwritten, + /// stranding a connected transport no later dispose could reach. + /// + [Fact] + public async Task A_second_InitializeAsync_disposes_the_first_transport() + { + var first = new FakeTransport(); + var second = new FakeTransport(); + var handed = new List(); + var opts = new ModbusDriverOptions + { + Host = "fake", + Probe = new ModbusProbeOptions { Enabled = false }, + RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)]), + }; + var drv = new ModbusDriver(opts, "modbus-1", _ => + { + var next = handed.Count == 0 ? first : second; + handed.Add(next); + return next; + }); + + await drv.InitializeAsync("{}", CancellationToken.None); + // No ShutdownAsync in between — exactly what AdoptConfigDuringInit does. + await drv.InitializeAsync("{}", CancellationToken.None); + + handed.Count.ShouldBe(2, "each initialize must build its own transport"); + first.DisposeCount.ShouldBe(1, "the first initialize's transport must be disposed by the second initialize"); + second.DisposeCount.ShouldBe(0, "the live transport must not be disposed"); + + // …and the driver is still functional, serving reads through the SECOND transport. + drv.GetHealth().State.ShouldNotBe(DriverState.Faulted); + second.HoldingRegisters[0] = 7; + var r = await drv.ReadAsync(["A"], CancellationToken.None); + r[0].StatusCode.ShouldBe(0u); + Convert.ToInt32(r[0].Value).ShouldBe(7); + second.SendCount.ShouldBeGreaterThan(0, "reads must route through the transport the second initialize published"); + } + + /// + /// The probe / re-probe CancellationTokenSources are re-assigned by every initialize, so a + /// second initialize must cancel the previous pair or their background loops run forever + /// against a transport that is already gone. + /// + [Fact] + public async Task A_second_InitializeAsync_stops_the_first_probe_and_reprobe_loops() + { + var first = new FakeTransport(); + var second = new FakeTransport(); + var handed = 0; + var opts = new ModbusDriverOptions + { + Host = "fake", + Probe = new ModbusProbeOptions + { + Enabled = true, + Interval = TimeSpan.FromMilliseconds(50), + Timeout = TimeSpan.FromSeconds(1), + }, + AutoProhibitReprobeInterval = TimeSpan.FromMilliseconds(50), + RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)]), + }; + var drv = new ModbusDriver(opts, "modbus-1", _ => Interlocked.Increment(ref handed) == 1 ? first : second); + + await drv.InitializeAsync("{}", CancellationToken.None); + await Task.Delay(200); + Interlocked.CompareExchange(ref first.SendCount, 0, 0).ShouldBeGreaterThan(0, + "the first initialize's probe loop should have issued at least one send"); + + await drv.InitializeAsync("{}", CancellationToken.None); + + // The first transport's loops must stop; tolerate one in-flight send straddling the cancel. + var atReinit = Interlocked.CompareExchange(ref first.SendCount, 0, 0); + await Task.Delay(400); + var atRest = Interlocked.CompareExchange(ref first.SendCount, 0, 0); + (atRest - atReinit).ShouldBeLessThanOrEqualTo(1, + "the previous initialize's probe/re-probe loops must be cancelled by the next initialize"); + + await drv.DisposeAsync(); + } + // -------------------- Finding -005 / -012 (3) -------------------- ///