dohertj2/lmxopcua
1
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(modbus): dispose the previous transport and probe loops on re-initialize

Browse Source
This commit is contained in:
Joseph Doherty
2026-08-15 21:37:28 -04:00
parent dc459633e0
commit c3970a2e5d
2 changed files with 111 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
+24
View File
@@ -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 /
tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs
+87
View File
@@ -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 --------------------
/// <summary>
/// A second <c>InitializeAsync</c> on a live driver — the shape
/// <c>DriverInstanceActor.AdoptConfigDuringInit</c> 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 <c>_transport</c> was simply overwritten,
/// stranding a connected transport no later dispose could reach.
/// </summary>
[Fact]
public async Task A_second_InitializeAsync_disposes_the_first_transport()
{
var first = new FakeTransport();
var second = new FakeTransport();
var handed = new List<FakeTransport>();
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");
}
/// <summary>
/// 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.
/// </summary>
[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) --------------------
/// <summary>