Joseph Doherty
8ef5adc63a
v2-ci / unit-tests (push) Waiting to run
Reinitialize_clears_lastPublished_and_lastWritten_caches now registers ShutdownAsync through the shared FixtureCleanup helper before initialize, so an initialize, seeding/polling-observation or assertion failure still stops the probe loop and poll engine and disposes the transport; the primary failure is preserved and a secondary cleanup failure is reported. The in-body shutdown still precedes the cache-clearing assertions; the trailing cleanup is safe because shutdown is idempotent. Default probe settings, the real subscription path and the bounded deadline are unchanged. New Cache_reset_fixture_failure_after_subscribe_still_shuts_the_driver_down fails the body after the subscription is live and asserts the sentinel is kept, the transport is disposed exactly once, caches are cleared and no further poll or probe sends land (fails if the cleanup is removed). No production change.
746 lines
38 KiB
C#
746 lines
38 KiB
C#
using System.Collections.Concurrent;
|
|
using System.Reflection;
|
|
using Shouldly;
|
|
using ZB.MOM.WW.OtOpcUa.Tests;
|
|
using Xunit;
|
|
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
|
using ZB.MOM.WW.OtOpcUa.Driver.Modbus;
|
|
|
|
namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
|
|
|
|
/// <summary>
|
|
/// Regression coverage for Driver.Modbus findings -002 (Reinitialize state hygiene),
|
|
/// -003 (_health volatile-write ordering), -004 (DisposeAsync teardown parity), and
|
|
/// -005 (malformed/short response PDU handling). All four resolved fixes need a
|
|
/// unit test alongside them per Driver.Modbus-012.
|
|
/// </summary>
|
|
[Trait("Category", "Unit")]
|
|
public sealed class ModbusLifecycleHygieneTests(ITestOutputHelper output)
|
|
{
|
|
private sealed class FakeTransport : IModbusTransport
|
|
{
|
|
public readonly ushort[] HoldingRegisters = new ushort[256];
|
|
public int ConnectCount;
|
|
public int DisposeCount;
|
|
public int SendCount;
|
|
|
|
/// <summary>Establishes a connection asynchronously.</summary>
|
|
/// <param name="ct">Cancellation token for the operation.</param>
|
|
public Task ConnectAsync(CancellationToken ct) { Interlocked.Increment(ref ConnectCount); return Task.CompletedTask; }
|
|
|
|
/// <summary>Sends a Modbus PDU and receives the response.</summary>
|
|
/// <param name="unitId">The Modbus unit identifier.</param>
|
|
/// <param name="pdu">The Protocol Data Unit to send.</param>
|
|
/// <param name="ct">Cancellation token for the operation.</param>
|
|
public Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
|
|
{
|
|
Interlocked.Increment(ref SendCount);
|
|
var fc = pdu[0];
|
|
switch (fc)
|
|
{
|
|
case 0x03:
|
|
case 0x04:
|
|
{
|
|
var addr = (ushort)((pdu[1] << 8) | pdu[2]);
|
|
var qty = (ushort)((pdu[3] << 8) | pdu[4]);
|
|
var resp = new byte[2 + qty * 2];
|
|
resp[0] = fc;
|
|
resp[1] = (byte)(qty * 2);
|
|
for (var i = 0; i < qty; i++)
|
|
{
|
|
resp[2 + i * 2] = (byte)(HoldingRegisters[addr + i] >> 8);
|
|
resp[3 + i * 2] = (byte)(HoldingRegisters[addr + i] & 0xFF);
|
|
}
|
|
return Task.FromResult(resp);
|
|
}
|
|
case 0x06:
|
|
{
|
|
var addr = (ushort)((pdu[1] << 8) | pdu[2]);
|
|
HoldingRegisters[addr] = (ushort)((pdu[3] << 8) | pdu[4]);
|
|
return Task.FromResult(pdu); // FC06 echoes the request
|
|
}
|
|
case 0x10:
|
|
{
|
|
var addr = (ushort)((pdu[1] << 8) | pdu[2]);
|
|
var qty = (ushort)((pdu[3] << 8) | pdu[4]);
|
|
for (var i = 0; i < qty; i++)
|
|
HoldingRegisters[addr + i] = (ushort)((pdu[6 + i * 2] << 8) | pdu[7 + i * 2]);
|
|
return Task.FromResult(new byte[] { 0x10, pdu[1], pdu[2], pdu[3], pdu[4] });
|
|
}
|
|
default:
|
|
return Task.FromException<byte[]>(new NotSupportedException($"fc={fc}"));
|
|
}
|
|
}
|
|
|
|
/// <summary>Disposes the transport asynchronously.</summary>
|
|
public ValueTask DisposeAsync() { Interlocked.Increment(ref DisposeCount); return ValueTask.CompletedTask; }
|
|
}
|
|
|
|
/// <summary>
|
|
/// Returns a snapshot of the driver's private <c>_tagsByRawPath</c> dictionary so the
|
|
/// hygiene tests can confirm the cache is empty after teardown.
|
|
/// </summary>
|
|
private static System.Collections.IDictionary GetTagsByName(ModbusDriver drv) =>
|
|
(System.Collections.IDictionary)typeof(ModbusDriver)
|
|
.GetField("_tagsByRawPath", BindingFlags.NonPublic | BindingFlags.Instance)!
|
|
.GetValue(drv)!;
|
|
|
|
// -------------------- Finding -002 / -012 (2) --------------------
|
|
|
|
/// <summary>Verifies that reinitialization clears stale tag cache entries.</summary>
|
|
[Fact]
|
|
public async Task Reinitialize_clears_stale_tagsByName_entries()
|
|
{
|
|
// Re-initializing with a different options instance would leak stale entries before
|
|
// the fix. We simulate by inspecting _tagsByName after a Shutdown — it must be empty
|
|
// so InitializeAsync repopulates from a clean slate.
|
|
var fake = new FakeTransport();
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)]),
|
|
};
|
|
var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
GetTagsByName(drv).Count.ShouldBe(1);
|
|
await drv.ShutdownAsync(CancellationToken.None);
|
|
GetTagsByName(drv).Count.ShouldBe(0, "Shutdown must clear the tag cache so the next Initialize starts clean");
|
|
}
|
|
|
|
private static ModbusDriverOptions CacheResetOptions() => new()
|
|
{
|
|
Host = "fake",
|
|
WriteOnChangeOnly = true,
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16,
|
|
Deadband: 1.0)]),
|
|
};
|
|
|
|
private static System.Collections.IDictionary GetCache(ModbusDriver drv, string field) =>
|
|
(System.Collections.IDictionary)typeof(ModbusDriver)
|
|
.GetField(field, BindingFlags.NonPublic | BindingFlags.Instance)!
|
|
.GetValue(drv)!;
|
|
|
|
/// <summary>
|
|
/// Seeds the write-suppression cache with a write and the deadband cache through a real subscription,
|
|
/// waiting (bounded) for the poll engine to publish. Returns the live subscription handle.
|
|
/// </summary>
|
|
private static async Task<ISubscriptionHandle> SeedCachesAsync(ModbusDriver drv, FakeTransport fake)
|
|
{
|
|
var lastPublished = GetCache(drv, "_lastPublishedByRef");
|
|
var lastWritten = GetCache(drv, "_lastWrittenByRef");
|
|
|
|
// Seed both caches via a write (lastWritten) and a publish through ShouldPublish (lastPublished).
|
|
await drv.WriteAsync([new WriteRequest("A", (short)5)], CancellationToken.None);
|
|
lastWritten.Count.ShouldBe(1);
|
|
|
|
// Reach ShouldPublish directly through a subscription so the deadband cache fills.
|
|
fake.HoldingRegisters[0] = 5;
|
|
var handle = await drv.SubscribeAsync(["A"], TimeSpan.FromMilliseconds(100), CancellationToken.None);
|
|
var deadline = DateTime.UtcNow.AddSeconds(2);
|
|
while (lastPublished.Count == 0 && DateTime.UtcNow < deadline) await Task.Delay(25, TestContext.Current.CancellationToken);
|
|
lastPublished.Count.ShouldBe(1);
|
|
return handle;
|
|
}
|
|
|
|
/// <summary>Verifies that reinitialization clears the deadband and write-suppression caches.</summary>
|
|
/// <remarks>
|
|
/// #727: cleanup is registered before initialize, so an initialize, seeding or assertion failure still
|
|
/// shuts the driver down (probe loop, poll engine, transport). The in-body ShutdownAsync stays ahead of
|
|
/// the cache-clearing assertions it is the oracle for; the registered cleanup runs after it and is safe
|
|
/// because shutdown is idempotent.
|
|
/// </remarks>
|
|
[Fact]
|
|
public async Task Reinitialize_clears_lastPublished_and_lastWritten_caches()
|
|
{
|
|
var fake = new FakeTransport();
|
|
var drv = new ModbusDriver(CacheResetOptions(), "modbus-1", _ => fake);
|
|
await FixtureCleanup.RunAsync(async () =>
|
|
{
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
var lastPublished = GetCache(drv, "_lastPublishedByRef");
|
|
var lastWritten = GetCache(drv, "_lastWrittenByRef");
|
|
|
|
var handle = await SeedCachesAsync(drv, fake);
|
|
await drv.UnsubscribeAsync(handle, CancellationToken.None);
|
|
|
|
await drv.ShutdownAsync(CancellationToken.None);
|
|
lastPublished.Count.ShouldBe(0, "Shutdown must clear the deadband cache");
|
|
lastWritten.Count.ShouldBe(0, "Shutdown must clear the write-suppression cache");
|
|
}, () => drv.ShutdownAsync(CancellationToken.None),
|
|
failure => output.WriteLine($"Modbus cache-reset fixture cleanup failed: {failure}"));
|
|
}
|
|
|
|
/// <summary>
|
|
/// #727 failure path: a body that fails after the subscription and probe loop are live keeps its
|
|
/// primary exception, and the registered cleanup still releases the fixture — the transport is
|
|
/// disposed once, the caches are cleared, and no further polling or probe sends land.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task Cache_reset_fixture_failure_after_subscribe_still_shuts_the_driver_down()
|
|
{
|
|
var fake = new FakeTransport();
|
|
var drv = new ModbusDriver(CacheResetOptions(), "modbus-1", _ => fake);
|
|
var sentinel = new InvalidOperationException("cache-reset body sentinel");
|
|
|
|
var actual = await Should.ThrowAsync<InvalidOperationException>(() => FixtureCleanup.RunAsync(async () =>
|
|
{
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
_ = await SeedCachesAsync(drv, fake);
|
|
fake.DisposeCount.ShouldBe(0);
|
|
throw sentinel; // no Unsubscribe, no Shutdown — the leak shape the fixture used to have
|
|
}, () => drv.ShutdownAsync(CancellationToken.None),
|
|
failure => output.WriteLine($"Modbus cache-reset fixture cleanup failed: {failure}")));
|
|
|
|
actual.ShouldBeSameAs(sentinel);
|
|
actual.Data.Contains(FixtureCleanup.SecondaryExceptionKey).ShouldBeFalse("cleanup itself must succeed");
|
|
fake.DisposeCount.ShouldBe(1, "cleanup must dispose the transport exactly once");
|
|
GetCache(drv, "_lastPublishedByRef").Count.ShouldBe(0);
|
|
GetCache(drv, "_lastWrittenByRef").Count.ShouldBe(0);
|
|
drv.GetHealth().State.ShouldNotBe(DriverState.Healthy);
|
|
|
|
// Poll engine and probe loop are both stopped: the send count is flat across several poll intervals.
|
|
var sendsAfterCleanup = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
|
|
await Task.Delay(300, TestContext.Current.CancellationToken);
|
|
Interlocked.CompareExchange(ref fake.SendCount, 0, 0).ShouldBe(sendsAfterCleanup,
|
|
"no subscription poll or probe may run after the failure-path cleanup");
|
|
}
|
|
|
|
// -------------------- Finding -004 / -012 (4) --------------------
|
|
|
|
/// <summary>Verifies that DisposeAsync without Shutdown stops probe loops and tears down the transport.</summary>
|
|
[Fact]
|
|
public async Task DisposeAsync_without_explicit_Shutdown_tears_down_probe_loop_and_transport()
|
|
{
|
|
var fake = new FakeTransport();
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
Probe = new ModbusProbeOptions
|
|
{
|
|
Enabled = true,
|
|
Interval = TimeSpan.FromMilliseconds(50),
|
|
Timeout = TimeSpan.FromSeconds(1),
|
|
},
|
|
// Re-probe loop also opted in so DisposeAsync exercises both CTS cancellations.
|
|
AutoProhibitReprobeInterval = TimeSpan.FromMilliseconds(50),
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)]),
|
|
};
|
|
var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
// Wait for the probe loop to issue its first send instead of sleeping a fixed 200 ms — on a
|
|
// loaded CI runner the loop had not run yet and the precondition below read 0 (#606). This is a
|
|
// presence wait (returns as soon as a send lands); the post-dispose absence window stays fixed.
|
|
var deadline = DateTime.UtcNow.AddSeconds(10);
|
|
while (Interlocked.CompareExchange(ref fake.SendCount, 0, 0) == 0 && DateTime.UtcNow < deadline)
|
|
await Task.Delay(10, TestContext.Current.CancellationToken);
|
|
var sendsAtDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
|
|
sendsAtDispose.ShouldBeGreaterThan(0, "background probe loop should have issued at least one send");
|
|
|
|
// Skip ShutdownAsync — exercise the await-using path that previously leaked.
|
|
await drv.DisposeAsync();
|
|
|
|
// Transport must have been disposed exactly once and the background loops stop scheduling
|
|
// new sends. Tolerate at most one in-flight send straddling the cancel.
|
|
fake.DisposeCount.ShouldBe(1);
|
|
var sendsAfterDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
|
|
await Task.Delay(300);
|
|
var sendsAtRest = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
|
|
(sendsAtRest - sendsAfterDispose).ShouldBeLessThanOrEqualTo(1, "background loops must stop after DisposeAsync");
|
|
}
|
|
|
|
/// <summary>Verifies that DisposeAsync disposes the poll engine so subscriptions stop.</summary>
|
|
[Fact]
|
|
public async Task DisposeAsync_disposes_the_pollEngine_so_subscriptions_stop()
|
|
{
|
|
var fake = new 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", _ => fake);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
// Spin up a polled subscription; the PollGroupEngine schedules a background Task that
|
|
// will keep issuing SendAsync until either Unsubscribe or DisposeAsync stops it.
|
|
var handle = await drv.SubscribeAsync(["A"], TimeSpan.FromMilliseconds(100), CancellationToken.None);
|
|
await Task.Delay(250);
|
|
var beforeDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
|
|
beforeDispose.ShouldBeGreaterThan(0);
|
|
|
|
// No ShutdownAsync — DisposeAsync must also tear down the poll engine.
|
|
await drv.DisposeAsync();
|
|
|
|
var atDispose = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
|
|
await Task.Delay(400);
|
|
var atRest = Interlocked.CompareExchange(ref fake.SendCount, 0, 0);
|
|
|
|
(atRest - atDispose).ShouldBeLessThanOrEqualTo(1,
|
|
"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 with no teardown between (the <c>IDriver</c>
|
|
/// contract allows it) 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.
|
|
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>
|
|
/// <c>_probeCts</c> is re-assigned by every initialize, so a second initialize must cancel the
|
|
/// previous one. <c>ProbeLoopAsync</c> CAPTURES its transport reference at loop entry, so a
|
|
/// leaked probe loop keeps hammering the transport the first initialize published — which is
|
|
/// exactly what this measures. Re-probe is off here so every send is attributable to the probe.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task A_second_InitializeAsync_stops_the_first_probe_loop()
|
|
{
|
|
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(25),
|
|
Timeout = TimeSpan.FromSeconds(1),
|
|
},
|
|
AutoProhibitReprobeInterval = null, // re-probe off: isolate the probe loop
|
|
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,
|
|
"precondition: the first initialize's probe loop must actually be sending");
|
|
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
// Tolerate one in-flight send straddling the cancel; a leaked loop produces ~16 in 400ms.
|
|
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 loop must be cancelled by the next initialize");
|
|
|
|
await drv.DisposeAsync();
|
|
}
|
|
|
|
/// <summary>
|
|
/// Transport for the re-probe test: counts every send and fails one address permanently, so a
|
|
/// seeded prohibition on that address is never cleared and the re-probe loop keeps re-probing
|
|
/// it tick after tick. A transport that answered normally would let the very first re-probe
|
|
/// CLEAR the prohibition, emptying the candidate set and making the test vacuous.
|
|
/// </summary>
|
|
private sealed class ReprobeCountingTransport : IModbusTransport
|
|
{
|
|
/// <summary>Register address whose reads always fail.</summary>
|
|
public ushort FailAddress { get; init; }
|
|
/// <summary>Total sends this transport has been asked to perform.</summary>
|
|
public int SendCount;
|
|
|
|
/// <summary>Establishes a connection asynchronously.</summary>
|
|
/// <param name="ct">Cancellation token for the operation.</param>
|
|
public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask;
|
|
|
|
/// <summary>Counts the send, failing reads aimed at <see cref="FailAddress"/>.</summary>
|
|
/// <param name="unitId">The Modbus unit identifier.</param>
|
|
/// <param name="pdu">The Protocol Data Unit to send.</param>
|
|
/// <param name="ct">Cancellation token for the operation.</param>
|
|
public Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
|
|
{
|
|
Interlocked.Increment(ref SendCount);
|
|
var addr = (ushort)((pdu[1] << 8) | pdu[2]);
|
|
if (addr == FailAddress)
|
|
return Task.FromException<byte[]>(new IOException($"address {addr} is unreadable"));
|
|
return Task.FromResult(new byte[] { pdu[0], 2, 0, 0 });
|
|
}
|
|
|
|
/// <summary>Disposes the transport asynchronously.</summary>
|
|
public ValueTask DisposeAsync() => ValueTask.CompletedTask;
|
|
}
|
|
|
|
/// <summary>Reads a private instance field off the driver.</summary>
|
|
private static object GetPrivateField(ModbusDriver drv, string name) =>
|
|
typeof(ModbusDriver).GetField(name, BindingFlags.NonPublic | BindingFlags.Instance)!.GetValue(drv)!;
|
|
|
|
/// <summary>
|
|
/// <c>_reprobeCts</c> is re-assigned by every initialize, so a second initialize must cancel the
|
|
/// previous one. Two things make this measurable where the probe test's approach would not:
|
|
/// the re-probe loop only sends when the prohibition set is NON-EMPTY (so one is seeded, on an
|
|
/// address the transport fails permanently), and it re-reads the <c>_transport</c> FIELD every
|
|
/// pass rather than capturing it — so a leaked loop hits the transport the SECOND initialize
|
|
/// published. The second initialize is therefore given re-probe-less options: any send at all on
|
|
/// the second transport is the leaked loop from the first.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task A_second_InitializeAsync_stops_the_first_reprobe_loop()
|
|
{
|
|
const ushort prohibited = 100;
|
|
var first = new ReprobeCountingTransport { FailAddress = prohibited };
|
|
var second = new ReprobeCountingTransport { FailAddress = prohibited };
|
|
var handed = 0;
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
Probe = new ModbusProbeOptions { Enabled = false }, // probe off: isolate the re-probe loop
|
|
AutoProhibitReprobeInterval = TimeSpan.FromMilliseconds(25),
|
|
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);
|
|
|
|
// Seed a single-register prohibition BEFORE the first initialize, so no loop is running while
|
|
// the (unsynchronised) dictionary is mutated. Start == End keeps it off the bisection path, so
|
|
// each tick is exactly one StraightReprobeAsync send.
|
|
var prohibitions = (System.Collections.IDictionary)GetPrivateField(drv, "_autoProhibited");
|
|
var stateType = typeof(ModbusDriver).GetNestedType("ProhibitionState", BindingFlags.NonPublic)!;
|
|
prohibitions.Add(
|
|
((byte)1, ModbusRegion.HoldingRegisters, prohibited, prohibited),
|
|
Activator.CreateInstance(stateType, nonPublic: true)!);
|
|
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
await Task.Delay(200);
|
|
Interlocked.CompareExchange(ref first.SendCount, 0, 0).ShouldBeGreaterThan(0,
|
|
"precondition: the seeded prohibition must make the re-probe loop actually send");
|
|
|
|
// The second initialize must NOT start a re-probe loop of its own, or a leaked first loop and a
|
|
// healthy second one would be indistinguishable by send count. "{}" keeps these options.
|
|
typeof(ModbusDriver).GetField("_options", BindingFlags.NonPublic | BindingFlags.Instance)!
|
|
.SetValue(drv, new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
Probe = new ModbusProbeOptions { Enabled = false },
|
|
AutoProhibitReprobeInterval = null,
|
|
RawTags = opts.RawTags,
|
|
});
|
|
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
// Non-vacuity guard: the candidate set the leaked loop would iterate must still be non-empty,
|
|
// so a zero send count is attributable to the cancel and not to an emptied prohibition set.
|
|
prohibitions.Count.ShouldBe(1, "a re-initialize must not clear the prohibition set");
|
|
|
|
var atReinit = Interlocked.CompareExchange(ref second.SendCount, 0, 0);
|
|
await Task.Delay(400);
|
|
var atRest = Interlocked.CompareExchange(ref second.SendCount, 0, 0);
|
|
(atRest - atReinit).ShouldBeLessThanOrEqualTo(1,
|
|
"the previous initialize's re-probe loop must be cancelled by the next initialize");
|
|
|
|
await drv.DisposeAsync();
|
|
}
|
|
|
|
/// <summary>
|
|
/// Two initializes racing on the same driver — the shape <c>DriverInstanceActor</c> produces by
|
|
/// dispatching initialize on a fire-and-forget <c>Task.Run</c>, where a reconnect retry tick can
|
|
/// overlap a still-running attempt — must serialize on the lifecycle gate. Ungated, the teardown at the top of one
|
|
/// initialize disposes the transport the other just published (or nulls <c>_transport</c>
|
|
/// mid-connect), leaving the driver holding a disposed transport.
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task Concurrent_InitializeAsync_calls_serialize_and_leave_exactly_one_live_transport()
|
|
{
|
|
var handed = new ConcurrentQueue<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 t = new FakeTransport();
|
|
handed.Enqueue(t);
|
|
return t;
|
|
});
|
|
|
|
await Task.WhenAll(
|
|
Task.Run(() => drv.InitializeAsync("{}", CancellationToken.None)),
|
|
Task.Run(() => drv.InitializeAsync("{}", CancellationToken.None)));
|
|
|
|
var built = handed.ToArray();
|
|
built.Length.ShouldBe(2);
|
|
// Serialized: the earlier initialize's transport is disposed by the later one, and exactly one
|
|
// survives — never two live transports, and never a disposed transport left installed.
|
|
built.Count(t => t.DisposeCount == 0).ShouldBe(1, "exactly one transport must be left live");
|
|
built.Count(t => t.DisposeCount == 1).ShouldBe(1, "the superseded transport must be disposed once");
|
|
|
|
// The survivor is the installed one and the driver still works through it.
|
|
var live = built.Single(t => t.DisposeCount == 0);
|
|
live.HoldingRegisters[0] = 9;
|
|
var r = await drv.ReadAsync(["A"], CancellationToken.None);
|
|
r[0].StatusCode.ShouldBe(0u);
|
|
Convert.ToInt32(r[0].Value).ShouldBe(9);
|
|
|
|
await drv.DisposeAsync();
|
|
}
|
|
|
|
// -------------------- Finding -005 / -012 (3) --------------------
|
|
|
|
/// <summary>
|
|
/// Transport that returns a structurally-broken response for FC03/FC04 — too short to
|
|
/// hold the declared byte-count. Pre-fix the driver dereferenced <c>resp[1]</c> and then
|
|
/// ran <c>Buffer.BlockCopy(resp, 2, ..., resp[1])</c> which threw <c>ArgumentException</c>
|
|
/// (out-of-range). Post-fix the driver throws <c>InvalidDataException</c> which the
|
|
/// <c>ReadAsync</c> catch-all maps to <see cref="BadCommunicationError"/>.
|
|
/// </summary>
|
|
private sealed class TruncatingTransport : IModbusTransport
|
|
{
|
|
/// <summary>How many bytes to return — anything < 2 + bytecount is malformed.</summary>
|
|
public int ResponseBytes { get; set; } = 1; // just the fc byte, no bytecount
|
|
|
|
/// <summary>Establishes a connection asynchronously.</summary>
|
|
/// <param name="ct">Cancellation token for the operation.</param>
|
|
public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask;
|
|
/// <summary>Sends a Modbus PDU and receives the response.</summary>
|
|
/// <param name="unitId">The Modbus unit identifier.</param>
|
|
/// <param name="pdu">The Protocol Data Unit to send.</param>
|
|
/// <param name="ct">Cancellation token for the operation.</param>
|
|
public Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
|
|
{
|
|
var resp = new byte[ResponseBytes];
|
|
if (ResponseBytes >= 1) resp[0] = pdu[0];
|
|
if (ResponseBytes >= 2) resp[1] = 4; // claim 4 bytes of payload but provide none
|
|
return Task.FromResult(resp);
|
|
}
|
|
/// <summary>Disposes the transport asynchronously.</summary>
|
|
public ValueTask DisposeAsync() => ValueTask.CompletedTask;
|
|
}
|
|
|
|
/// <summary>Verifies that short response PDUs surface as BadCommunicationError, not IndexOutOfRangeException.</summary>
|
|
[Fact]
|
|
public async Task Short_response_PDU_surfaces_as_BadCommunicationError_not_an_IndexOutOfRangeException()
|
|
{
|
|
var fake = new TruncatingTransport { ResponseBytes = 1 };
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("Level", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)]),
|
|
};
|
|
var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
var r = await drv.ReadAsync(["Level"], CancellationToken.None);
|
|
r[0].StatusCode.ShouldBe(0x80050000u, "BadCommunicationError = a clean transport-layer fault");
|
|
r[0].Value.ShouldBeNull();
|
|
}
|
|
|
|
/// <summary>Verifies that response payloads truncated below declared byte count surface as BadCommunicationError.</summary>
|
|
[Fact]
|
|
public async Task Response_payload_truncated_below_declared_byteCount_surfaces_as_BadCommunicationError()
|
|
{
|
|
// Header says "4 bytes follow" but the message is only 3 bytes total — pre-fix the
|
|
// Buffer.BlockCopy would throw ArgumentException.
|
|
var fake = new TruncatingTransport { ResponseBytes = 3 };
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("Level", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)]),
|
|
};
|
|
var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
var r = await drv.ReadAsync(["Level"], CancellationToken.None);
|
|
r[0].StatusCode.ShouldBe(0x80050000u);
|
|
}
|
|
|
|
/// <summary>Verifies that DecodeBitArray rejects an empty bitmap with InvalidDataException.</summary>
|
|
[Fact]
|
|
public async Task DecodeBitArray_rejects_an_empty_bitmap_with_InvalidDataException()
|
|
{
|
|
var decode = typeof(ModbusDriver).GetMethod(
|
|
"DecodeBitArray", BindingFlags.NonPublic | BindingFlags.Static)!;
|
|
// We can't invoke through reflection because ReadOnlySpan<byte> isn't representable in
|
|
// object-array invocation parameters. Instead, exercise the path through ReadAsync with
|
|
// a bit-region tag and a transport that returns a zero-byte-count response.
|
|
var fake = new EmptyBitTransport();
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("Coil", ModbusRegion.Coils, 0, ModbusDataType.Bool)]),
|
|
};
|
|
var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
|
|
await FixtureCleanup.RunAsync(async () =>
|
|
{
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
var r = await drv.ReadAsync(["Coil"], CancellationToken.None);
|
|
// The empty-bitmap guard surfaces via the BadCommunicationError catch-all.
|
|
r[0].StatusCode.ShouldBe(0x80050000u);
|
|
}, () => drv.ShutdownAsync(CancellationToken.None),
|
|
failure => output.WriteLine($"Modbus empty bitmap cleanup failed: {failure}"));
|
|
}
|
|
|
|
[Fact]
|
|
public async Task Body_failure_after_probe_request_cancels_it_and_disposes_transport()
|
|
{
|
|
var fake = new EmptyBitTransport();
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("Coil", ModbusRegion.Coils, 0, ModbusDataType.Bool)]),
|
|
};
|
|
var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
|
|
var sentinel = new InvalidOperationException("body sentinel");
|
|
try
|
|
{
|
|
var actual = await Should.ThrowAsync<InvalidOperationException>(() => FixtureCleanup.RunAsync(async () =>
|
|
{
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
var result = await drv.ReadAsync(["Coil"], CancellationToken.None);
|
|
result[0].StatusCode.ShouldBe(0x80050000u);
|
|
fake.CoilRequestCount.ShouldBe(1);
|
|
await fake.ProbeEntered.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
|
|
fake.ProbeToken.IsCancellationRequested.ShouldBeFalse();
|
|
fake.DisposeCount.ShouldBe(0);
|
|
throw sentinel;
|
|
}, () => drv.ShutdownAsync(CancellationToken.None),
|
|
failure => output.WriteLine($"Modbus probe fixture cleanup failed: {failure}")));
|
|
actual.ShouldBeSameAs(sentinel);
|
|
fake.ProbeToken.IsCancellationRequested.ShouldBeTrue();
|
|
await fake.ProbeCancelled.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
|
|
await fake.ProbeRequestCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
|
|
fake.DisposeCount.ShouldBe(1);
|
|
}
|
|
finally { fake.ReleaseProbe.TrySetResult(); }
|
|
}
|
|
|
|
/// <summary>
|
|
/// Coil-bank transport that returns <c>[fc][bytecount=0]</c> — a response with a
|
|
/// declared zero-byte payload. Pre-fix <c>DecodeBitArray</c> indexed into the empty
|
|
/// bitmap and threw <c>IndexOutOfRangeException</c>.
|
|
/// </summary>
|
|
private sealed class EmptyBitTransport : IModbusTransport
|
|
{
|
|
public TaskCompletionSource ProbeEntered { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
|
|
public TaskCompletionSource ProbeCancelled { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
|
|
public TaskCompletionSource ProbeRequestCompleted { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
|
|
public TaskCompletionSource ReleaseProbe { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
|
|
public CancellationToken ProbeToken { get; private set; }
|
|
public int DisposeCount { get; private set; }
|
|
public int CoilRequestCount { get; private set; }
|
|
|
|
/// <summary>Returns a completed task without performing any connection.</summary>
|
|
/// <param name="ct">The cancellation token for the operation.</param>
|
|
public Task ConnectAsync(CancellationToken ct) => Task.CompletedTask;
|
|
/// <summary>Returns a response with zero-byte payload to simulate empty bitmap.</summary>
|
|
/// <param name="unitId">The Modbus unit ID.</param>
|
|
/// <param name="pdu">The protocol data unit being sent.</param>
|
|
/// <param name="ct">The cancellation token for the operation.</param>
|
|
public async Task<byte[]> SendAsync(byte unitId, byte[] pdu, CancellationToken ct)
|
|
{
|
|
if (pdu[0] == 0x03)
|
|
{
|
|
ProbeToken = ct;
|
|
ProbeEntered.TrySetResult();
|
|
try { await ReleaseProbe.Task.WaitAsync(ct); }
|
|
catch (OperationCanceledException) when (ct.IsCancellationRequested)
|
|
{
|
|
ProbeCancelled.TrySetResult();
|
|
throw;
|
|
}
|
|
finally { ProbeRequestCompleted.TrySetResult(); }
|
|
}
|
|
if (pdu[0] == 0x01) CoilRequestCount++;
|
|
return new byte[] { pdu[0], 0 };
|
|
}
|
|
/// <summary>Records transport disposal.</summary>
|
|
public ValueTask DisposeAsync()
|
|
{
|
|
DisposeCount++;
|
|
return ValueTask.CompletedTask;
|
|
}
|
|
}
|
|
|
|
// -------------------- Finding -003 (volatile _health) --------------------
|
|
|
|
/// <summary>
|
|
/// The <c>_health</c> field is read by <c>GetHealth()</c> and written by every read /
|
|
/// write / probe path. The fix uses <c>Volatile.Read</c>/<c>Volatile.Write</c> to give
|
|
/// <c>GetHealth()</c> a defined ordering guarantee. We verify that under concurrent
|
|
/// pressure <c>GetHealth()</c> never returns a half-constructed value (it's a sealed
|
|
/// record so reference-assignment atomicity already prevents tearing; the test guards
|
|
/// against future regressions to a struct-typed health surface).
|
|
/// </summary>
|
|
[Fact]
|
|
public async Task GetHealth_under_concurrent_pressure_always_returns_a_complete_snapshot()
|
|
{
|
|
var fake = new FakeTransport();
|
|
var opts = new ModbusDriverOptions
|
|
{
|
|
Host = "fake",
|
|
RawTags = ModbusRawTags.Entries([new ModbusTagDefinition("A", ModbusRegion.HoldingRegisters, 0, ModbusDataType.Int16)]),
|
|
};
|
|
var drv = new ModbusDriver(opts, "modbus-1", _ => fake);
|
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
|
|
|
// Two writer loops and one reader loop — 250ms of churn.
|
|
var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(250));
|
|
var faults = new ConcurrentQueue<Exception>();
|
|
var writer = Task.Run(async () =>
|
|
{
|
|
try { while (!cts.IsCancellationRequested) await drv.ReadAsync(["A"], CancellationToken.None); }
|
|
catch (Exception ex) { faults.Enqueue(ex); }
|
|
});
|
|
var reader = Task.Run(() =>
|
|
{
|
|
try
|
|
{
|
|
while (!cts.IsCancellationRequested)
|
|
{
|
|
var h = drv.GetHealth();
|
|
// State must be one of the enum values; LastSuccessfulRead can be null or a real time;
|
|
// the record constructor enforces no field is wholly garbage.
|
|
h.State.ShouldBeOneOf(DriverState.Unknown, DriverState.Initializing, DriverState.Healthy, DriverState.Degraded, DriverState.Faulted);
|
|
}
|
|
}
|
|
catch (Exception ex) { faults.Enqueue(ex); }
|
|
});
|
|
|
|
await Task.WhenAll(writer, reader);
|
|
faults.ShouldBeEmpty();
|
|
}
|
|
}
|