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

fix(runtime): derive the write cap from the configured write timeout (lmxopcua#774)

Browse Source 
DriverInstanceActor.RunWriteAsync cancelled every write with a hard-coded,
undilated 5 s CancellationTokenSource, so writeTimeout / DefaultWriteTimeout
(then 10 s) only ever served as the pessimistic abandon bound: a configured
bound above 5 s never took effect.

The write's token is now cancelled at the configured write timeout; a write
that ignores its token is abandoned WriteAbandonGrace later (half the
timeout, at most 2 s) so a token-honouring driver still gets to answer the
cancellation itself. Both bounds run on an injectable TimeProvider
(Props writeClock, System by default).

DefaultWriteTimeout is now 5 s (cancel 5 s, abandon 7 s), not 10 s: the
host's write Ask is 8 s, and a write allowed to run past it would already
have been reported as a timeout and its node reverted while the device could
still take the value - the exact failure the off-mailbox write work fixed.
Production behaviour is therefore unchanged for the cancel (5 s) and tighter
for a token-ignoring driver (abandoned at 7 s instead of 10 s, i.e. inside
the Ask). DriverHostActor's Ask comment updated to match.

Tests (DriverInstanceActorConcurrentWriteTests, manual clock copied from
Ab.Transport.Tests into Runtime.Tests/Harness/ManualTimeProvider):
- a write held past 6 s under a 10 s write timeout succeeds;
- positive control: at 10 s the token is cancelled ("write timeout");
- a token-ignoring write is still pending at 10 s and abandoned (BadTimeout)
  after the grace.
Mutation checks: a fixed 5 s token on the same clock fails the first test;
dropping the grace fails the third (and the second, where the abandon then
races the cancellation).

342a825d re-check: its held-write tests were written around the undilated
5 s cap. Connect() now defaults the write timeout to the DILATED presence
budget, so a held, explicitly-released write no longer races any undilated
deadline; tests about the bound pass their own. AwaitWritesEntered keeps its
20 ms interval (doc updated).

Runtime.Tests: 946 passed / 3 skipped / 0 failed at AKKA_TEST_TIMEFACTOR 3
and 1. Solution Release build: 0 warnings.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 15:18:37 -05:00
parent f0992394b6
commit 75be6fe12b
4 changed files with 259 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorConcurrentWriteTests.cs
+99 -5
View File
@@ -26,21 +26,31 @@ public sealed class DriverInstanceActorConcurrentWriteTests : RuntimeActorTestBa
/// Polls for <paramref name="count"/> writes inside the driver with an EXPLICIT short interval. Akka's
/// <c>AwaitCondition</c> without one polls every <c>max / 10</c> of the DILATED budget — 1.5 s locally, 4.5 s
/// under CI's <c>AKKA_TEST_TIMEFACTOR=3</c>. The write under test is already held inside the driver by then,
/// and <c>DriverInstanceActor.RunWriteAsync</c> bounds it with a fixed (undilated) 5 s token, so one missed
/// first poll plus the test's own settle window cancelled it ("write timeout") before
/// and it was bounded by <c>DriverInstanceActor.RunWriteAsync</c>'s fixed (undilated) 5 s token, so one
/// missed first poll plus the test's own settle window cancelled it ("write timeout") before
/// <c>ReleaseWrites()</c>: <c>A_queued_write_older_than_the_budget…</c> failed every CI run that hit it
/// (hosted runs 2215 and 2260), and fails locally at timefactor 3.
/// (hosted runs 2215 and 2260), and failed locally at timefactor 3. Since lmxopcua#774 that token follows
/// the actor's write timeout, and <see cref="Connect"/> sets it to the dilated presence budget, so a held
/// write no longer races an undilated bound at all; the short interval stays to keep the tests fast.
/// </summary>
private void AwaitWritesEntered(SlowWritableDriver driver, int count) =>
AwaitCondition(() => driver.WritesEntered == count, PresenceBudget, TimeSpan.FromMilliseconds(20));
/// <summary>
/// Spawns and connects the actor. Unless a test passes its own <paramref name="writeTimeout"/>, the write
/// bound is the DILATED presence budget: these tests hold writes inside the driver and release them
/// explicitly, and are not about the bound — the production 5 s default would be an undilated deadline
/// racing every held write under CI's timefactor (the 342a825d failure mode).
/// </summary>
private (IActorRef Actor, Akka.TestKit.TestProbe Parent) Connect(
IDriver driver, int? maxWritesInFlight = null, TimeSpan? queuedWriteBudget = null, TimeSpan? writeTimeout = null)
IDriver driver, int? maxWritesInFlight = null, TimeSpan? queuedWriteBudget = null, TimeSpan? writeTimeout = null,
TimeProvider? writeClock = null)
{
var parent = CreateTestProbe();
var actor = parent.ChildActorOf(DriverInstanceActor.Props(
driver, commsLossTimeout: TimeSpan.Zero, maxWritesInFlight: maxWritesInFlight,
queuedWriteBudget: queuedWriteBudget, writeTimeout: writeTimeout));
queuedWriteBudget: queuedWriteBudget, writeTimeout: writeTimeout ?? Dilated(PresenceBudget),
writeClock: writeClock));
actor.Tell(new DriverInstanceActor.InitializeRequested("{}"));
parent.ExpectMsg<DriverInstanceActor.ConnectivityChanged>(m => m.Connected, PresenceBudget);
return (actor, parent);
@@ -213,6 +223,90 @@ public sealed class DriverInstanceActorConcurrentWriteTests : RuntimeActorTestBa
driver.Writes.Select(w => w.Value).ShouldBe(new object[] { 2 }, "only the later write reached the device's commit");
}
/// <summary>
/// lmxopcua#774: the write's cancellation token was a hard-coded 5 s, so a configured write timeout above
/// 5 s never took effect. On a manual clock: a write held past 6 s under a 10 s write timeout still
/// succeeds when the device answers. (Mutation-checked: a fixed 5 s token on the same clock fails this
/// with "write timeout".)
/// </summary>
[Fact]
public async Task A_write_longer_than_5s_but_within_the_configured_write_timeout_succeeds()
{
var clock = new ManualTimeProvider();
var driver = new SlowWritableDriver { HoldWrites = true };
var (actor, _) = Connect(driver, writeTimeout: TimeSpan.FromSeconds(10), writeClock: clock);
var write = actor.Ask<DriverInstanceActor.WriteAttributeResult>(
new DriverInstanceActor.WriteAttribute("tag-a", 1), PresenceBudget);
AwaitWritesEntered(driver, 1);
AwaitWriteBoundsArmed(clock);
clock.Advance(TimeSpan.FromSeconds(6));
await Task.Delay(100);
write.IsCompleted.ShouldBeFalse("the write was cut off before its configured 10 s bound");
driver.ReleaseWrites();
var result = await write;
result.Success.ShouldBeTrue(result.Reason);
driver.Writes.Select(w => w.FullReference).ShouldBe(new[] { "tag-a" });
}
/// <summary>
/// Positive control for the test above: the configured write timeout DOES bound the write — at 10 s its
/// token is cancelled and a token-honouring driver answers "write timeout" (not the abandon path).
/// </summary>
[Fact]
public async Task The_configured_write_timeout_cancels_the_write_when_it_elapses()
{
var clock = new ManualTimeProvider();
var driver = new SlowWritableDriver { HoldWrites = true };
var (actor, _) = Connect(driver, writeTimeout: TimeSpan.FromSeconds(10), writeClock: clock);
var write = actor.Ask<DriverInstanceActor.WriteAttributeResult>(
new DriverInstanceActor.WriteAttribute("tag-a", 1), PresenceBudget);
AwaitWritesEntered(driver, 1);
AwaitWriteBoundsArmed(clock);
clock.Advance(TimeSpan.FromSeconds(10));
var result = await write;
result.Success.ShouldBeFalse();
result.Reason.ShouldBe("write timeout");
driver.Writes.ShouldBeEmpty();
}
/// <summary>
/// A token-IGNORING write is abandoned <see cref="DriverInstanceActor.WriteAbandonGrace"/> after its token
/// is cancelled, not at the same instant (a token-honouring driver gets the grace to answer itself).
/// </summary>
[Fact]
public async Task A_token_ignoring_write_is_abandoned_a_grace_after_the_configured_write_timeout()
{
var clock = new ManualTimeProvider();
var driver = new SlowWritableDriver { HangFirstWriteIgnoringCancellation = true };
var (actor, _) = Connect(driver, writeTimeout: TimeSpan.FromSeconds(10), writeClock: clock);
var write = actor.Ask<DriverInstanceActor.WriteAttributeResult>(
new DriverInstanceActor.WriteAttribute("tag-a", 1), PresenceBudget);
AwaitWritesEntered(driver, 1);
AwaitWriteBoundsArmed(clock);
clock.Advance(TimeSpan.FromSeconds(10));
await Task.Delay(100);
write.IsCompleted.ShouldBeFalse("abandoned at the cancel instant — no grace for the driver to answer");
clock.Advance(DriverInstanceActor.WriteAbandonGrace(TimeSpan.FromSeconds(10)));
var result = await write;
result.Success.ShouldBeFalse();
result.Reason!.ShouldContain("800A0000", customMessage: "BadTimeout");
result.Reason!.ShouldContain("within 10000 ms");
}
/// <summary>Waits until the write task has armed BOTH of its bounds on <paramref name="clock"/> (the token
/// cancel and the abandon), so an Advance cannot run ahead of them. Counts the timers rather than matching their
/// due times, so a bound armed at the WRONG time fails the test's own assertion instead of this wait.</summary>
private void AwaitWriteBoundsArmed(ManualTimeProvider clock) =>
AwaitCondition(() => clock.ArmedTimers == 2, PresenceBudget, TimeSpan.FromMilliseconds(20));
[Fact]
public async Task Queued_writes_fail_fast_when_the_driver_leaves_Connected()
{
tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Harness/ManualTimeProvider.cs
+106
View File
@@ -0,0 +1,106 @@
namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness;
/// <summary>
/// Deterministic <see cref="TimeProvider"/>: time moves only on <see cref="Advance"/>, which fires every
/// timer falling due on the way, in due order, synchronously on the calling thread.
/// (Microsoft.Extensions.TimeProvider.Testing is not pinned in this repo; a copy of the Ab.Transport.Tests fake.)
/// </summary>
internal sealed class ManualTimeProvider : TimeProvider
{
private readonly object _lock = new();
private readonly List<ManualTimer> _timers = [];
private DateTimeOffset _now;
public ManualTimeProvider() => Start = _now = new DateTimeOffset(2026, 10, 7, 0, 0, 0, TimeSpan.Zero);
public DateTimeOffset Start { get; }
/// <summary>Steps the wall clock (<see cref="GetUtcNow"/>) without moving monotonic time
/// (<see cref="GetTimestamp"/>) or any timer, as an NTP or operator clock change does.</summary>
public TimeSpan WallClockOffset { get; set; }
public override DateTimeOffset GetUtcNow() { lock (_lock) return _now + WallClockOffset; }
public override long GetTimestamp() { lock (_lock) return _now.UtcTicks; }
public override long TimestampFrequency => TimeSpan.TicksPerSecond;
public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period)
{
var t = new ManualTimer(this, callback, state);
lock (_lock) _timers.Add(t);
t.Change(dueTime, period);
return t;
}
/// <summary>Armed timers (not disposed, with a due time).</summary>
public int ArmedTimers { get { lock (_lock) return _timers.Count(t => t.Due is not null); } }
public bool HasTimerDueAt(DateTimeOffset at) { lock (_lock) return _timers.Any(t => t.Due == at); }
/// <summary>Waits (in real time) until some code under test has armed a timer due at <paramref name="at"/>,
/// i.e. is genuinely parked on the fake clock rather than merely not yet scheduled.</summary>
public async Task WaitForTimerDueAtAsync(DateTimeOffset at, TimeSpan timeout)
{
var giveUp = DateTime.UtcNow + timeout;
while (!HasTimerDueAt(at))
{
if (DateTime.UtcNow > giveUp) throw new TimeoutException($"No timer armed for {at:O} (now {GetUtcNow():O}).");
await Task.Delay(5);
}
}
public void Advance(TimeSpan by)
{
DateTimeOffset target;
lock (_lock) target = _now + by;
while (true)
{
ManualTimer? next;
lock (_lock)
{
next = _timers.Where(t => t.Due <= target).MinBy(t => t.Due);
if (next is null) { _now = target; return; }
_now = next.Due!.Value;
next.Due = next.Period is { } p ? _now + p : null;
}
next.Fire();
}
}
private sealed class ManualTimer(ManualTimeProvider owner, TimerCallback callback, object? state) : ITimer
{
public DateTimeOffset? Due; // guarded by owner._lock
public TimeSpan? Period; // guarded by owner._lock
private bool _disposed;
public void Fire() => callback(state);
public bool Change(TimeSpan dueTime, TimeSpan period)
{
lock (owner._lock)
{
if (_disposed) return false;
Due = dueTime == Timeout.InfiniteTimeSpan ? null : owner._now + dueTime;
Period = period == Timeout.InfiniteTimeSpan || period == TimeSpan.Zero ? null : period;
return true;
}
}
public void Dispose()
{
lock (owner._lock)
{
_disposed = true;
Due = null;
owner._timers.Remove(this);
}
}
public ValueTask DisposeAsync()
{
Dispose();
return ValueTask.CompletedTask;
}
}
}