fix(ab-transport): per-pool diagnostics + per-device open-attempt limit, CIP-level close recycle, CipConnection/pool test gaps (#566, #581)

- Process-wide CipConnection statics (ForwardOpenCount, Forward Close counts, RequestsSent, exchange
  stats, OpenAttempts, OpenAttemptBudget) move to an instance ConnectionDiagnostics owned by each
  DeviceSessionPool (and by a stand-alone CipConnection). The open-attempt budget becomes a disposable
  per-DeviceKey limit (LimitOpenAttempts); the differential harness scopes one to the press under test.
- LibplctagOrphanRelease.ForceRootingForTest is gone: ReleaseIfOrphaned(tag, canReadHandles) overload.
- A connected reply with general 0x01 / extended 0x0107 or 0x0203 (target no longer holds the class-3
  connection, TCP still up) retires the session at once; the next send reopens through the throttle.
- New tests: dispose during RegisterSession, abandoned-open observer (falsified), scheduler-dispose-throws
  guard (DeviceSessionPool.CloseAsync), release-in-background with a shared open in flight, per-device
  limit scoping, CIP-close recycle (request + keep-alive + negative control).

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 23:21:02 -04:00
parent 501f6ea4e5
commit 3edff3304b
18 changed files with 695 additions and 188 deletions
+10
View File
@@ -172,6 +172,16 @@ The driver has two interchangeable wire transports, selected by one `DriverConfi
two types into one value. two types into one value.
- **Forward Open throttle:** at most one attempt in flight, at least 1 s between starts, bounded by the - **Forward Open throttle:** at most one attempt in flight, at least 1 s between starts, bounded by the
open budget. open budget.
- **A target-closed CIP connection is recycled at once (#566).** When a connected reply says the controller
no longer holds the class-3 connection while TCP stays up — general status `0x01` with extended `0x0107`
(target connection not found) or `0x0203` (connection timed out), e.g. after a controller restart or a
connection timeout — the session is closed (UnRegisterSession; no Forward Close, there is nothing to close)
and the next request reopens through the throttle. The request that got the reply fails
`BadCommunicationError` (transport `BadConnection`); a keep-alive that gets it recycles the connection
before any request fails. Every other status — tag-level errors, other `0x01` extended codes — is left to
the request. ⚠️ RESIDUAL: which of the two codes a real Logix controller returns for a forgotten
connection (and whether it answers at all rather than dropping TCP) is unobserved; the check is correct
whichever it does — a silent target still faults the session through the exchange timeout.
- `ConnectionSize` / `Timeout` / `AllowPacking` are honoured per pooled connection, and the **first** - `ConnectionSize` / `Timeout` / `AllowPacking` are honoured per pooled connection, and the **first**
driver/device to open a given device key wins — including a Test Connect probe on a fused `admin,driver` driver/device to open a given device key wins — including a Test Connect probe on a fused `admin,driver`
node (its connection carries the probe timeout). A later opener with different options is ignored and node (its connection carries the probe timeout). A later opener with different options is ignored and
@@ -47,10 +47,7 @@ internal static class LibplctagOrphanRelease
private static int _released; private static int _released;
/// <summary>True when the pinned wrapper's private fields were found, so a failed tag can be destroyed.</summary> /// <summary>True when the pinned wrapper's private fields were found, so a failed tag can be destroyed.</summary>
internal static bool CanReadHandles => HandleField is not null && !ForceRootingForTest; internal static bool CanReadHandles => HandleField is not null;
/// <summary>Test seam (review r1 M1): take the rooting fallback as if the wrapper's fields had been renamed.</summary>
internal static bool ForceRootingForTest { get; set; }
/// <summary>Orphaned native tags destroyed so far (process-wide; diagnostics and tests).</summary> /// <summary>Orphaned native tags destroyed so far (process-wide; diagnostics and tests).</summary>
internal static int Released => Volatile.Read(ref _released); internal static int Released => Volatile.Read(ref _released);
@@ -117,12 +114,19 @@ internal static class LibplctagOrphanRelease
/// </summary> /// </summary>
/// <returns>The status the native tag held just before it was destroyed (what <c>GetStatus()</c> would have /// <returns>The status the native tag held just before it was destroyed (what <c>GetStatus()</c> would have
/// reported), or null when nothing was destroyed.</returns> /// reported), or null when nothing was destroyed.</returns>
internal static Status? ReleaseIfOrphaned(Tag tag) internal static Status? ReleaseIfOrphaned(Tag tag) => ReleaseIfOrphaned(tag, CanReadHandles);
/// <summary>
/// <see cref="ReleaseIfOrphaned(Tag)"/> with the handle-readability decision passed in, so a test can take the
/// rooting fallback (review r1 M1) without a process-wide switch (Gitea #581). Production always passes
/// <see cref="CanReadHandles"/>.
/// </summary>
internal static Status? ReleaseIfOrphaned(Tag tag, bool canReadHandles)
{ {
try try
{ {
if (tag.IsInitialized) return null; if (tag.IsInitialized) return null;
if (!CanReadHandles) if (!canReadHandles)
{ {
Rooted.Add(tag); Rooted.Add(tag);
return null; return null;
@@ -10,7 +10,9 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
/// <remarks> /// <remarks>
/// <para><b>Reconnect.</b> Only a faulted session (timeout, I/O error, malformed frame) is reopened, and /// <para><b>Reconnect.</b> Only a faulted session (timeout, I/O error, malformed frame) is reopened, and
/// only by the <i>next</i> send; the failing call throws its own exception. Caller cancellation never /// only by the <i>next</i> send; the failing call throws its own exception. Caller cancellation never
/// faults the session (B2), so it never causes a reconnect.</para> /// faults the session (B2), so it never causes a reconnect. A connected reply saying the target no longer
/// holds the class-3 connection (general 0x01, extended 0x0107 / 0x0203 — the session itself still up) retires
/// the pair at once, whether a request or a keep-alive got it, so the next send reopens it (Gitea #566).</para>
/// <para><b>Bounded.</b> Every <see cref="SendAsync"/> is bounded even if the caller passes no token /// <para><b>Bounded.</b> Every <see cref="SendAsync"/> is bounded even if the caller passes no token
/// (ruling R9-3): opening (including the throttle wait and any wait behind another opener) is bounded by /// (ruling R9-3): opening (including the throttle wait and any wait behind another opener) is bounded by
/// <c>MinForwardOpenInterval + 4 × Timeout</c> (connect, RegisterSession, Forward Open, fallback Forward /// <c>MinForwardOpenInterval + 4 × Timeout</c> (connect, RegisterSession, Forward Open, fallback Forward
@@ -21,70 +23,6 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
/// </remarks> /// </remarks>
public sealed class CipConnection : ICipChannel, IAsyncDisposable public sealed class CipConnection : ICipChannel, IAsyncDisposable
{ {
/// <summary>Forward Open frames sent by every connection in the process (fallback included). Gate diagnostics.</summary>
internal static int ForwardOpenCount;
/// <summary>Forward Close requests put on the wire by every connection in the process. Gate diagnostics.</summary>
internal static int ForwardClosesSent;
/// <summary>Forward Close requests that got a reply within their budget. Gate diagnostics.</summary>
internal static int ForwardClosesAcked;
/// <summary>Channel sends by every connection in the process (keep-alives included). Gate diagnostics.</summary>
internal static int RequestsSent;
// Round-trip statistics of completed exchanges, process-wide (gate diagnostics: T21 latency attribution).
// Two Stopwatch reads and four interlocked ops per request; reset by the harness between timed passes.
private static long _exchangeCount;
private static long _exchangeTotalTicks;
private static long _exchangeMinTicks = long.MaxValue;
private static long _exchangeMaxTicks;
/// <summary>Round-trip statistics of the exchanges completed since the last <see cref="ResetExchangeStats"/>.</summary>
internal readonly record struct ExchangeStats(long Count, double MinMs, double MaxMs, double MeanMs);
/// <summary>Clears the process-wide round-trip statistics. Gate diagnostics.</summary>
internal static void ResetExchangeStats()
{
Interlocked.Exchange(ref _exchangeCount, 0);
Interlocked.Exchange(ref _exchangeTotalTicks, 0);
Interlocked.Exchange(ref _exchangeMinTicks, long.MaxValue);
Interlocked.Exchange(ref _exchangeMaxTicks, 0);
}
/// <summary>Snapshot of the process-wide round-trip statistics. Gate diagnostics.</summary>
internal static ExchangeStats ReadExchangeStats()
{
var count = Interlocked.Read(ref _exchangeCount);
if (count == 0) return new ExchangeStats(0, 0, 0, 0);
static double Ms(long ticks) => ticks * 1000.0 / Stopwatch.Frequency;
return new ExchangeStats(count, Ms(Interlocked.Read(ref _exchangeMinTicks)), Ms(Interlocked.Read(ref _exchangeMaxTicks)),
Ms(Interlocked.Read(ref _exchangeTotalTicks)) / count);
}
private static void RecordExchange(long ticks)
{
Interlocked.Increment(ref _exchangeCount);
Interlocked.Add(ref _exchangeTotalTicks, ticks);
long seen;
while (ticks < (seen = Interlocked.Read(ref _exchangeMinTicks))
&& Interlocked.CompareExchange(ref _exchangeMinTicks, ticks, seen) != seen) { }
while (ticks > (seen = Interlocked.Read(ref _exchangeMaxTicks))
&& Interlocked.CompareExchange(ref _exchangeMaxTicks, ticks, seen) != seen) { }
}
/// <summary>Open attempts (TCP connect + RegisterSession + Forward Open, a fallback open included) started by
/// every connection in the process — counted before any I/O, so a refused attempt counts too. Gate diagnostics.</summary>
internal static int OpenAttempts;
/// <summary>
/// Process-wide cap on <see cref="OpenAttempts"/>; <c>-1</c> (the default) is unlimited. An attempt beyond
/// the cap fails with <see cref="AbStatus.Abort"/> before any I/O — no TCP connect, no Forward Open. Only the
/// read-only differential harness (Task 18/21) sets it, so a production press that refuses or drops the
/// connection sees exactly one open attempt instead of one per remaining operation.
/// </summary>
internal static int OpenAttemptBudget = -1;
/// <summary>Get Attribute Single, Identity (class 1) instance 1 attribute 1 (Vendor ID).</summary> /// <summary>Get Attribute Single, Identity (class 1) instance 1 attribute 1 (Vendor ID).</summary>
internal static readonly byte[] KeepAliveRequest = [0x0E, 0x03, 0x20, 0x01, 0x24, 0x01, 0x30, 0x01]; internal static readonly byte[] KeepAliveRequest = [0x0E, 0x03, 0x20, 0x01, 0x24, 0x01, 0x30, 0x01];
@@ -98,6 +36,7 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
private readonly ForwardOpenThrottle _throttle; private readonly ForwardOpenThrottle _throttle;
private readonly TimeProvider _time; private readonly TimeProvider _time;
private readonly Task _predecessorClosed; private readonly Task _predecessorClosed;
private readonly ConnectionDiagnostics _diagnostics;
private readonly byte[] _route; private readonly byte[] _route;
private readonly object _sync = new(); private readonly object _sync = new();
@@ -119,9 +58,11 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
/// <param name="time">Clock for the throttle, deadlines and keep-alive.</param> /// <param name="time">Clock for the throttle, deadlines and keep-alive.</param>
/// <param name="predecessorClosed">Completes when the previous connection for the same key has /// <param name="predecessorClosed">Completes when the previous connection for the same key has
/// finished closing, so two connections to one device never overlap.</param> /// finished closing, so two connections to one device never overlap.</param>
/// <param name="diagnostics">The owning pool's counters and open-attempt limits; a private set when null.</param>
internal CipConnection(DeviceKey key, ConnectionOptions options, ForwardOpenThrottle throttle, TimeProvider time, internal CipConnection(DeviceKey key, ConnectionOptions options, ForwardOpenThrottle throttle, TimeProvider time,
Task? predecessorClosed = null) Task? predecessorClosed = null, ConnectionDiagnostics? diagnostics = null)
{ {
_diagnostics = diagnostics ?? new ConnectionDiagnostics();
_key = key ?? throw new ArgumentNullException(nameof(key)); _key = key ?? throw new ArgumentNullException(nameof(key));
_o = options ?? throw new ArgumentNullException(nameof(options)); _o = options ?? throw new ArgumentNullException(nameof(options));
_throttle = throttle ?? throw new ArgumentNullException(nameof(throttle)); _throttle = throttle ?? throw new ArgumentNullException(nameof(throttle));
@@ -141,6 +82,12 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
/// <summary>Test hook: keep-alives started (counted synchronously when the timer decides to send).</summary> /// <summary>Test hook: keep-alives started (counted synchronously when the timer decides to send).</summary>
internal int KeepAlivesSent => Volatile.Read(ref _keepAlivesSent); internal int KeepAlivesSent => Volatile.Read(ref _keepAlivesSent);
/// <summary>This connection's counters (its pool's, when it has one).</summary>
internal ConnectionDiagnostics Diagnostics => _diagnostics;
/// <summary>Test hook: true while a shared open is in progress.</summary>
internal bool OpenInProgress { get { lock (_sync) return _pendingOpen is not null; } }
private TimeSpan OpenBudget => _o.MinForwardOpenInterval + 4 * _o.Timeout; private TimeSpan OpenBudget => _o.MinForwardOpenInterval + 4 * _o.Timeout;
private TimeSpan ExchangeBudget => 2 * _o.Timeout; private TimeSpan ExchangeBudget => 2 * _o.Timeout;
@@ -210,6 +157,7 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
Volatile.Write(ref _open, null); Volatile.Write(ref _open, null);
} }
if (stale is not null) await stale.Client.DisposeAsync().ConfigureAwait(false); // faulted: no UnRegister if (stale is not null) await stale.Client.DisposeAsync().ConfigureAwait(false); // faulted: no UnRegister
if (!_predecessorClosed.IsCompleted) _diagnostics.CountPredecessorWait();
await _predecessorClosed.WaitAsync(beforeAttempt.Token).ConfigureAwait(false); await _predecessorClosed.WaitAsync(beforeAttempt.Token).ConfigureAwait(false);
opened = await _throttle.RunAsync(_ => OpenAsync(deadline.Token), beforeAttempt.Token).ConfigureAwait(false); opened = await _throttle.RunAsync(_ => OpenAsync(deadline.Token), beforeAttempt.Token).ConfigureAwait(false);
} }
@@ -235,11 +183,7 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
private async Task<Open> OpenAsync(CancellationToken ct) private async Task<Open> OpenAsync(CancellationToken ct)
{ {
ThrowIfDisposed(); // disposal may have begun while this attempt waited out the throttle ThrowIfDisposed(); // disposal may have begun while this attempt waited out the throttle
var attempt = Interlocked.Increment(ref OpenAttempts); if (_diagnostics.AdmitOpenAttempt(_key) is { } refused) throw new AbTransportException(AbStatus.Abort, refused);
var budget = Volatile.Read(ref OpenAttemptBudget);
if (budget >= 0 && attempt > budget)
throw new AbTransportException(AbStatus.Abort,
$"Open attempt {attempt} to {_key.Host}:{_key.Port} refused before any I/O: the process-wide open-attempt budget of {budget} is spent.");
var client = await EncapsulationClient.ConnectAsync(_key.Host, _key.Port, _o.Timeout, ct).ConfigureAwait(false); var client = await EncapsulationClient.ConnectAsync(_key.Host, _key.Port, _o.Timeout, ct).ConfigureAwait(false);
if (Volatile.Read(ref _disposed) != 0) if (Volatile.Read(ref _disposed) != 0)
{ {
@@ -276,7 +220,7 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
var p = new ForwardOpenParams(large, toId, serial, _o.VendorId, _o.OriginatorSerial, _o.TimeoutMultiplier, var p = new ForwardOpenParams(large, toId, serial, _o.VendorId, _o.OriginatorSerial, _o.TimeoutMultiplier,
_o.RpiMicros, size, _route); _o.RpiMicros, size, _route);
var frame = ConnectionManagerServices.ForwardOpen(p); var frame = ConnectionManagerServices.ForwardOpen(p);
Interlocked.Increment(ref ForwardOpenCount); _diagnostics.CountForwardOpen();
return CipReply.Parse(await client.SendRRDataAsync(frame, _o.Timeout, ct).ConfigureAwait(false)); return CipReply.Parse(await client.SendRRDataAsync(frame, _o.Timeout, ct).ConfigureAwait(false));
} }
@@ -289,7 +233,7 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
/// <summary>One request over an established pair, bounded by <see cref="ExchangeBudget"/>.</summary> /// <summary>One request over an established pair, bounded by <see cref="ExchangeBudget"/>.</summary>
private async Task<ReadOnlyMemory<byte>> ExchangeAsync(Open open, ReadOnlyMemory<byte> cip, CancellationToken ct) private async Task<ReadOnlyMemory<byte>> ExchangeAsync(Open open, ReadOnlyMemory<byte> cip, CancellationToken ct)
{ {
Interlocked.Increment(ref RequestsSent); _diagnostics.CountRequest();
Interlocked.Exchange(ref _lastSendTimestamp, _time.GetTimestamp()); Interlocked.Exchange(ref _lastSendTimestamp, _time.GetTimestamp());
using var deadline = new CancellationTokenSource(ExchangeBudget, _time); using var deadline = new CancellationTokenSource(ExchangeBudget, _time);
using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct, deadline.Token); using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct, deadline.Token);
@@ -302,7 +246,14 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
CipRouting.UnconnectedSend => await open.Client.SendRRDataAsync(ConnectionManagerServices.UnconnectedSend(cip.Span, _route), _o.Timeout, linked.Token).ConfigureAwait(false), CipRouting.UnconnectedSend => await open.Client.SendRRDataAsync(ConnectionManagerServices.UnconnectedSend(cip.Span, _route), _o.Timeout, linked.Token).ConfigureAwait(false),
_ => await open.Client.SendRRDataAsync(cip, _o.Timeout, linked.Token).ConfigureAwait(false), _ => await open.Client.SendRRDataAsync(cip, _o.Timeout, linked.Token).ConfigureAwait(false),
}; };
RecordExchange(Stopwatch.GetTimestamp() - started); _diagnostics.RecordExchange(Stopwatch.GetTimestamp() - started);
if (_o.Routing == CipRouting.Connected && ConnectionManagerServices.IsConnectionGone(reply.Span) is { } extended)
{
await RetireClosedByTargetAsync(open).ConfigureAwait(false);
throw new AbTransportException(AbStatus.BadConnection,
$"{_key.Host}:{_key.Port} no longer holds the CIP connection (general status 0x01, extended 0x{extended:X4}); " +
"the session was closed and the next request reopens it.");
}
return reply; return reply;
} }
catch (OperationCanceledException) when (!ct.IsCancellationRequested && deadline.IsCancellationRequested) catch (OperationCanceledException) when (!ct.IsCancellationRequested && deadline.IsCancellationRequested)
@@ -312,6 +263,24 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
} }
} }
/// <summary>
/// The target answered over a session that is still up, but says the class-3 connection itself is gone (it
/// timed the connection out, or was restarted / re-downloaded and forgot it). Every later connected request
/// would get the same answer while <see cref="EncapsulationClient.IsConnected"/> stays true, so nothing else
/// would ever reopen it (Gitea #566): retire the pair now, so the next send reopens through the throttle.
/// No Forward Close — the target has just said there is nothing to close.
/// </summary>
private async Task RetireClosedByTargetAsync(Open open)
{
lock (_sync)
{
if (!ReferenceEquals(_open, open)) return; // a concurrent exchange (keep-alive) already retired it
Volatile.Write(ref _open, null);
}
_diagnostics.CountClosedByTarget();
await open.Client.DisposeAsync().ConfigureAwait(false); // UnRegisterSession, then close the socket
}
/// <summary>One-shot timer, re-armed after every tick so the longest idle gap is one /// <summary>One-shot timer, re-armed after every tick so the longest idle gap is one
/// <see cref="ConnectionOptions.KeepAliveInterval"/> (half the connection expiry), not two.</summary> /// <see cref="ConnectionOptions.KeepAliveInterval"/> (half the connection expiry), not two.</summary>
private void OnKeepAliveTimer() private void OnKeepAliveTimer()
@@ -396,11 +365,11 @@ public sealed class CipConnection : ICipChannel, IAsyncDisposable
{ {
var budget = TimeSpan.FromSeconds(1); var budget = TimeSpan.FromSeconds(1);
using var cts = new CancellationTokenSource(budget, _time); using var cts = new CancellationTokenSource(budget, _time);
Interlocked.Increment(ref ForwardClosesSent); _diagnostics.CountForwardCloseSent();
await open.Client.SendRRDataAsync( await open.Client.SendRRDataAsync(
ConnectionManagerServices.ForwardClose(open.ConnectionSerial, _o.VendorId, _o.OriginatorSerial, _route), ConnectionManagerServices.ForwardClose(open.ConnectionSerial, _o.VendorId, _o.OriginatorSerial, _route),
budget, cts.Token).ConfigureAwait(false); budget, cts.Token).ConfigureAwait(false);
Interlocked.Increment(ref ForwardClosesAcked); _diagnostics.CountForwardCloseAcked();
} }
catch (Exception ex) when (ex is AbTransportException or OperationCanceledException) catch (Exception ex) when (ex is AbTransportException or OperationCanceledException)
{ {
@@ -0,0 +1,163 @@
using System.Diagnostics;
namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
/// <summary>
/// Counters and the open-attempt limit for the <see cref="CipConnection"/>s one <see cref="DeviceSessionPool"/>
/// owns. Gate and harness diagnostics: nothing in production reads them, and a limit is only ever set by the
/// read-only differential harness.
/// </summary>
/// <remarks>
/// Instance-scoped (Gitea #581): every pool — and every connection a test builds on its own — has its own, so
/// no process-wide mutable state ships in the transport, and two tests can never move each other's counts. The
/// open-attempt limit is scoped to one <see cref="DeviceKey"/> (Gitea #566), so limiting the press under test
/// can never refuse an open to any other device in the process.
/// </remarks>
internal sealed class ConnectionDiagnostics
{
private readonly object _lock = new();
private readonly Dictionary<DeviceKey, OpenAttemptLimit> _limits = new();
private int _forwardOpens;
private int _forwardClosesSent;
private int _forwardClosesAcked;
private int _requestsSent;
private int _openAttempts;
private int _predecessorWaits;
private int _connectionsClosedByTarget;
// Round-trip statistics of completed exchanges (T21 latency attribution). Two Stopwatch reads and four
// interlocked ops per request; reset by the harness between timed passes.
private long _exchangeCount;
private long _exchangeTotalTicks;
private long _exchangeMinTicks = long.MaxValue;
private long _exchangeMaxTicks;
/// <summary>Forward Open frames sent (fallback included).</summary>
public int ForwardOpens => Volatile.Read(ref _forwardOpens);
/// <summary>Forward Close requests put on the wire.</summary>
public int ForwardClosesSent => Volatile.Read(ref _forwardClosesSent);
/// <summary>Forward Close requests that got a reply within their budget.</summary>
public int ForwardClosesAcked => Volatile.Read(ref _forwardClosesAcked);
/// <summary>Channel sends (keep-alives included).</summary>
public int RequestsSent => Volatile.Read(ref _requestsSent);
/// <summary>Open attempts (TCP connect + RegisterSession + Forward Open, a fallback open included), counted
/// before any I/O, so an attempt refused by a limit counts too.</summary>
public int OpenAttempts => Volatile.Read(ref _openAttempts);
/// <summary>Opens that found an earlier connection for the same device still closing and parked behind it.</summary>
public int PredecessorWaits => Volatile.Read(ref _predecessorWaits);
/// <summary>Connections retired because the target reported the CIP connection itself gone (TCP still up).</summary>
public int ConnectionsClosedByTarget => Volatile.Read(ref _connectionsClosedByTarget);
/// <summary>Zeroes the Forward Open / Forward Close counters (the harness, before its native phase).</summary>
public void ResetForwardCounts()
{
Interlocked.Exchange(ref _forwardOpens, 0);
Interlocked.Exchange(ref _forwardClosesSent, 0);
Interlocked.Exchange(ref _forwardClosesAcked, 0);
}
internal void CountForwardOpen() => Interlocked.Increment(ref _forwardOpens);
internal void CountForwardCloseSent() => Interlocked.Increment(ref _forwardClosesSent);
internal void CountForwardCloseAcked() => Interlocked.Increment(ref _forwardClosesAcked);
internal void CountRequest() => Interlocked.Increment(ref _requestsSent);
internal void CountPredecessorWait() => Interlocked.Increment(ref _predecessorWaits);
internal void CountClosedByTarget() => Interlocked.Increment(ref _connectionsClosedByTarget);
/// <summary>Round-trip statistics of the exchanges completed since the last <see cref="ResetExchangeStats"/>.</summary>
internal readonly record struct ExchangeStats(long Count, double MinMs, double MaxMs, double MeanMs);
/// <summary>Clears the round-trip statistics.</summary>
public void ResetExchangeStats()
{
Interlocked.Exchange(ref _exchangeCount, 0);
Interlocked.Exchange(ref _exchangeTotalTicks, 0);
Interlocked.Exchange(ref _exchangeMinTicks, long.MaxValue);
Interlocked.Exchange(ref _exchangeMaxTicks, 0);
}
/// <summary>Snapshot of the round-trip statistics.</summary>
public ExchangeStats ReadExchangeStats()
{
var count = Interlocked.Read(ref _exchangeCount);
if (count == 0) return new ExchangeStats(0, 0, 0, 0);
static double Ms(long ticks) => ticks * 1000.0 / Stopwatch.Frequency;
return new ExchangeStats(count, Ms(Interlocked.Read(ref _exchangeMinTicks)), Ms(Interlocked.Read(ref _exchangeMaxTicks)),
Ms(Interlocked.Read(ref _exchangeTotalTicks)) / count);
}
internal void RecordExchange(long ticks)
{
Interlocked.Increment(ref _exchangeCount);
Interlocked.Add(ref _exchangeTotalTicks, ticks);
long seen;
while (ticks < (seen = Interlocked.Read(ref _exchangeMinTicks))
&& Interlocked.CompareExchange(ref _exchangeMinTicks, ticks, seen) != seen) { }
while (ticks > (seen = Interlocked.Read(ref _exchangeMaxTicks))
&& Interlocked.CompareExchange(ref _exchangeMaxTicks, ticks, seen) != seen) { }
}
/// <summary>
/// Caps the open attempts to <paramref name="key"/> at <paramref name="attempts"/> until the returned scope is
/// disposed. An attempt beyond the cap fails with <see cref="AbStatus.Abort"/> before any I/O — no TCP connect,
/// no Forward Open — so a press that refuses or drops the connection sees exactly the budgeted attempts instead
/// of one per remaining operation. Only the read-only differential harness (Task 18/21) sets one; one limit per
/// key at a time.
/// </summary>
public OpenAttemptLimit LimitOpenAttempts(DeviceKey key, int attempts)
{
ArgumentNullException.ThrowIfNull(key);
ArgumentOutOfRangeException.ThrowIfNegative(attempts);
var limit = new OpenAttemptLimit(this, key, attempts);
lock (_lock)
{
if (!_limits.TryAdd(key, limit))
throw new InvalidOperationException($"An open-attempt limit for {key.Host}:{key.Port} is already in force.");
}
return limit;
}
/// <summary>True while a limit is in force for <paramref name="key"/>.</summary>
public bool HasOpenAttemptLimit(DeviceKey key) { lock (_lock) return _limits.ContainsKey(key); }
/// <summary>Counts one open attempt to <paramref name="key"/>; null when it may proceed, else why it may not.</summary>
internal string? AdmitOpenAttempt(DeviceKey key)
{
var attempt = Interlocked.Increment(ref _openAttempts);
OpenAttemptLimit? limit;
lock (_lock) _limits.TryGetValue(key, out limit);
return limit is null || limit.TryTake()
? null
: $"Open attempt {attempt} to {key.Host}:{key.Port} refused before any I/O: the open-attempt budget of {limit.Budget} for this device is spent.";
}
private void Remove(OpenAttemptLimit limit)
{
lock (_lock)
{
if (_limits.TryGetValue(limit.Key, out var current) && current == limit) _limits.Remove(limit.Key);
}
}
/// <summary>An open-attempt cap on one device; disposing it lifts the cap.</summary>
internal sealed class OpenAttemptLimit(ConnectionDiagnostics owner, DeviceKey key, int budget) : IDisposable
{
private int _attempts;
public DeviceKey Key { get; } = key;
public int Budget { get; } = budget;
/// <summary>Attempts counted against this limit, the refused ones included.</summary>
public int Attempts => Volatile.Read(ref _attempts);
internal bool TryTake() => Interlocked.Increment(ref _attempts) <= Budget;
public void Dispose() => owner.Remove(this);
}
}
@@ -83,6 +83,26 @@ public static class ConnectionManagerServices
r.GeneralStatus == 0x08 r.GeneralStatus == 0x08
|| (r.GeneralStatus == 0x01 && r.ExtendedStatus.Length > 0 && r.ExtendedStatus[0] == 0x0109); || (r.GeneralStatus == 0x01 && r.ExtendedStatus.Length > 0 && r.ExtendedStatus[0] == 0x0109);
/// <summary>Connection Manager extended status: target connection not found.</summary>
public const ushort TargetConnectionNotFound = 0x0107;
/// <summary>Connection Manager extended status: the connection timed out.</summary>
public const ushort ConnectionTimedOut = 0x0203;
/// <summary>
/// For a reply received over a class-3 connection: the extended status when the target says the connection
/// itself no longer exists — general 0x01 (Connection Failure) with extended 0x0107 (target connection not
/// found) or 0x0203 (connection timed out) — else null. Any other status, a tag-level one included, describes
/// the request and leaves the connection alone.
/// </summary>
public static ushort? IsConnectionGone(ReadOnlySpan<byte> reply)
{
// Reply header: service | reserved | general status | extended-status word count | words...
if (reply.Length < 6 || reply[2] != 0x01 || reply[3] == 0) return null;
var extended = (ushort)(reply[4] | (reply[5] << 8));
return extended is TargetConnectionNotFound or ConnectionTimedOut ? extended : null;
}
public static byte[] ForwardClose(ushort connectionSerial, ushort vendorId, uint originatorSerial, ReadOnlySpan<byte> route) public static byte[] ForwardClose(ushort connectionSerial, ushort vendorId, uint originatorSerial, ReadOnlySpan<byte> route)
{ {
var connPath = new CipWriter().Bytes(route).Bytes(CipPath.MessageRouter).ToArray(); var connPath = new CipWriter().Bytes(route).Bytes(CipPath.MessageRouter).ToArray();
@@ -13,6 +13,8 @@ public sealed class ForwardOpenThrottle
private readonly TimeSpan _minInterval; private readonly TimeSpan _minInterval;
private readonly TimeProvider _time; private readonly TimeProvider _time;
private long? _lastStartTimestamp; // monotonic (TimeProvider.GetTimestamp); only touched while holding _inFlight private long? _lastStartTimestamp; // monotonic (TimeProvider.GetTimestamp); only touched while holding _inFlight
private int _queued;
private Task? _intervalWait; // the pending interval wait (test hook only)
/// <remarks>Internal: the only legitimate owner is <see cref="DeviceSessionPool"/>, which keeps exactly /// <remarks>Internal: the only legitimate owner is <see cref="DeviceSessionPool"/>, which keeps exactly
/// one per device. A second throttle for the same device would halve the guaranteed interval.</remarks> /// one per device. A second throttle for the same device would halve the guaranteed interval.</remarks>
@@ -24,6 +26,13 @@ public sealed class ForwardOpenThrottle
_time = time ?? throw new ArgumentNullException(nameof(time)); _time = time ?? throw new ArgumentNullException(nameof(time));
} }
/// <summary>Test hook: callers waiting behind an in-flight attempt.</summary>
internal int Queued => Volatile.Read(ref _queued);
/// <summary>Test hook: true while the next attempt is parked on the minimum interval and that wait has not
/// yet elapsed — no attempt is then in flight, and none can start until the clock moves.</summary>
internal bool IsWaitingOutInterval => Volatile.Read(ref _intervalWait) is { IsCompleted: false };
/// <summary>Run one open attempt once the throttle allows it. <paramref name="ct"/> bounds only the wait /// <summary>Run one open attempt once the throttle allows it. <paramref name="ct"/> bounds only the wait
/// BEFORE the attempt (queued behind an in-flight attempt, or the interval): when it fires there, this /// BEFORE the attempt (queued behind an in-flight attempt, or the interval): when it fires there, this
/// throws <see cref="AbTransportException"/>(<see cref="AbStatus.BadConnection"/>, "Forward Open /// throws <see cref="AbTransportException"/>(<see cref="AbStatus.BadConnection"/>, "Forward Open
@@ -33,9 +42,11 @@ public sealed class ForwardOpenThrottle
public async Task<T> RunAsync<T>(Func<CancellationToken, Task<T>> open, CancellationToken ct) public async Task<T> RunAsync<T>(Func<CancellationToken, Task<T>> open, CancellationToken ct)
{ {
ArgumentNullException.ThrowIfNull(open); ArgumentNullException.ThrowIfNull(open);
Interlocked.Increment(ref _queued);
try { await _inFlight.WaitAsync(ct).ConfigureAwait(false); } try { await _inFlight.WaitAsync(ct).ConfigureAwait(false); }
catch (OperationCanceledException) when (ct.IsCancellationRequested) catch (OperationCanceledException) when (ct.IsCancellationRequested)
{ throw Throttled("another attempt is in flight"); } { throw Throttled("another attempt is in flight"); }
finally { Interlocked.Decrement(ref _queued); }
try try
{ {
@@ -45,9 +56,12 @@ public sealed class ForwardOpenThrottle
var wait = _minInterval - _time.GetElapsedTime(last); var wait = _minInterval - _time.GetElapsedTime(last);
if (wait > TimeSpan.Zero) if (wait > TimeSpan.Zero)
{ {
try { await Task.Delay(wait, _time, ct).ConfigureAwait(false); } var delay = Task.Delay(wait, _time, ct);
Volatile.Write(ref _intervalWait, delay);
try { await delay.ConfigureAwait(false); }
catch (OperationCanceledException) when (ct.IsCancellationRequested) catch (OperationCanceledException) when (ct.IsCancellationRequested)
{ throw Throttled("the minimum interval had not elapsed"); } { throw Throttled("the minimum interval had not elapsed"); }
finally { Volatile.Write(ref _intervalWait, null); }
} }
} }
_lastStartTimestamp = _time.GetTimestamp(); _lastStartTimestamp = _time.GetTimestamp();
@@ -52,6 +52,9 @@ public sealed class DeviceSessionPool
/// <summary>Wiring-test hook: the logger this pool currently writes to (final review I2).</summary> /// <summary>Wiring-test hook: the logger this pool currently writes to (final review I2).</summary>
internal ILogger Logger => _logger; internal ILogger Logger => _logger;
/// <summary>Counters and open-attempt limits for this pool's connections (gate and harness diagnostics).</summary>
internal ConnectionDiagnostics Diagnostics { get; } = new();
/// <summary>Test hook: keys with a live entry.</summary> /// <summary>Test hook: keys with a live entry.</summary>
internal int EntryCount { get { lock (_lock) return _entries.Count; } } internal int EntryCount { get { lock (_lock) return _entries.Count; } }
@@ -69,7 +72,7 @@ public sealed class DeviceSessionPool
if (!_throttles.TryGetValue(key, out var throttle)) if (!_throttles.TryGetValue(key, out var throttle))
_throttles[key] = throttle = new ForwardOpenThrottle(options.MinForwardOpenInterval, _time); _throttles[key] = throttle = new ForwardOpenThrottle(options.MinForwardOpenInterval, _time);
var predecessor = _closing.GetValueOrDefault(key) ?? Task.CompletedTask; var predecessor = _closing.GetValueOrDefault(key) ?? Task.CompletedTask;
var connection = new CipConnection(key, options, throttle, _time, predecessor); var connection = new CipConnection(key, options, throttle, _time, predecessor, Diagnostics);
entry = new Entry(connection, new RequestScheduler(connection, options.AllowPacking), options, predecessor); entry = new Entry(connection, new RequestScheduler(connection, options.AllowPacking), options, predecessor);
_entries[key] = entry; _entries[key] = entry;
} }
@@ -109,28 +112,36 @@ public sealed class DeviceSessionPool
} }
}, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
// Outside the lock: scheduler first (fails its waiters, stops sending), then Forward Close + // Outside the lock.
// UnRegisterSession via the connection. The connection is disposed even if the scheduler's
// dispose throws: otherwise its keep-alive would hold the press connection open forever.
try try
{ {
try { await entry.Scheduler.DisposeAsync().ConfigureAwait(false); } await CloseAsync(key, entry.Scheduler, entry.Connection, _logger).ConfigureAwait(false);
catch (Exception ex)
{
_logger.LogWarning(ex, "Disposing the request scheduler for {Host}:{Port} failed; closing the connection anyway.",
key.Host, key.Port);
}
try { await entry.Connection.DisposeAsync().ConfigureAwait(false); }
catch (Exception ex)
{
_logger.LogWarning(ex, "Closing the connection to {Host}:{Port} failed.", key.Host, key.Port);
}
} }
finally finally
{ {
closed.TrySetResult(); // never faults: a successor only needs to know the close is over closed.TrySetResult(); // never faults: a successor only needs to know the close is over
} }
} }
/// <summary>
/// Scheduler first (fails its waiters, stops sending), then Forward Close + UnRegisterSession via the
/// connection. The connection is disposed even if the scheduler's dispose throws: otherwise its keep-alive
/// would hold the press connection open forever. Never throws; each failure is logged.
/// </summary>
internal static async Task CloseAsync(DeviceKey key, IAsyncDisposable scheduler, IAsyncDisposable connection, ILogger logger)
{
try { await scheduler.DisposeAsync().ConfigureAwait(false); }
catch (Exception ex)
{
logger.LogWarning(ex, "Disposing the request scheduler for {Host}:{Port} failed; closing the connection anyway.",
key.Host, key.Port);
}
try { await connection.DisposeAsync().ConfigureAwait(false); }
catch (Exception ex)
{
logger.LogWarning(ex, "Closing the connection to {Host}:{Port} failed.", key.Host, key.Port);
}
}
} }
/// <summary>A reference on a device's shared connection. Dispose exactly once (further disposes are no-ops).</summary> /// <summary>A reference on a device's shared connection. Dispose exactly once (further disposes are no-ops).</summary>
@@ -2,11 +2,14 @@ using System.Buffers.Binary;
using Shouldly; using Shouldly;
using Xunit; using Xunit;
using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip; using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Eip;
using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Fakes; using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Fakes;
namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Cip; namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Cip;
/// <summary>Serializes every test class that asserts on <see cref="CipConnection"/>'s process-wide /// <summary>Serializes the loopback-socket test classes. Their diagnostic counters are per connection / per
/// diagnostic counters, so no parallel test can move them mid-assertion.</summary> /// pool since Gitea #581 (no longer process-wide statics), but <see cref="DeviceSessionPoolTests"/> still
/// reconfigures <see cref="DeviceSessionPool.Shared"/>'s logger, and the timing-sensitive storm test is kept
/// off a thread pool its neighbours are saturating.</summary>
[CollectionDefinition(Name, DisableParallelization = true)] [CollectionDefinition(Name, DisableParallelization = true)]
public sealed class CipConnectionStatics public sealed class CipConnectionStatics
{ {
@@ -66,7 +69,7 @@ public sealed class CipConnectionTests
target.ForwardOpenReply = fo => fo[0] == 0x5B ? FakeCipTarget.Refuse(0x08)(fo) : FakeCipTarget.Accept(fo); target.ForwardOpenReply = fo => fo[0] == 0x5B ? FakeCipTarget.Refuse(0x08)(fo) : FakeCipTarget.Accept(fo);
var time = new ManualTimeProvider(); var time = new ManualTimeProvider();
await using var conn = Create(target, Options(), time); await using var conn = Create(target, Options(), time);
var before = Volatile.Read(ref CipConnection.ForwardOpenCount); var before = conn.Diagnostics.ForwardOpens;
await conn.SendAsync(ReadTag, TestCt); await conn.SendAsync(ReadTag, TestCt);
@@ -76,7 +79,7 @@ public sealed class CipConnectionTests
fos[1][0].ShouldBe((byte)0x54); fos[1][0].ShouldBe((byte)0x54);
RegularOTParams(fos[1]).ShouldBe((ushort)(0x4200 | 504)); RegularOTParams(fos[1]).ShouldBe((ushort)(0x4200 | 504));
conn.MaxRequestBytes.ShouldBe(502); conn.MaxRequestBytes.ShouldBe(502);
(Volatile.Read(ref CipConnection.ForwardOpenCount) - before).ShouldBe(2); (conn.Diagnostics.ForwardOpens - before).ShouldBe(2);
target.ConnectedRequests.ShouldBe(1); target.ConnectedRequests.ShouldBe(1);
} }
@@ -109,14 +112,12 @@ public sealed class CipConnectionTests
[Fact] [Fact]
public async Task A_spent_open_attempt_budget_refuses_the_reconnect_before_any_io() public async Task A_spent_open_attempt_budget_refuses_the_reconnect_before_any_io()
{ {
// Task 18 review r1 (C1): the differential harness caps the process at one open attempt, so a press // Task 18 review r1 (C1): the differential harness caps the press at one open attempt, so a press
// that refuses the Forward Open sees one TCP connect and one Forward Open, never one per remaining tag. // that refuses the Forward Open sees one TCP connect and one Forward Open, never one per remaining tag.
await using var target = new FakeCipTarget { ForwardOpenReply = FakeCipTarget.Refuse(0x01, 0x0100) }; await using var target = new FakeCipTarget { ForwardOpenReply = FakeCipTarget.Refuse(0x01, 0x0100) };
var time = new ManualTimeProvider(); var time = new ManualTimeProvider();
await using var conn = Create(target, Options(size: 504), time); await using var conn = Create(target, Options(size: 504), time);
var before = Volatile.Read(ref CipConnection.OpenAttempts); using (conn.Diagnostics.LimitOpenAttempts(Key(target), 1))
Volatile.Write(ref CipConnection.OpenAttemptBudget, before + 1);
try
{ {
await Should.ThrowAsync<AbTransportException>(() => conn.SendAsync(ReadTag, TestCt)); await Should.ThrowAsync<AbTransportException>(() => conn.SendAsync(ReadTag, TestCt));
var retry = conn.SendAsync(ReadTag, TestCt); var retry = conn.SendAsync(ReadTag, TestCt);
@@ -128,13 +129,43 @@ public sealed class CipConnectionTests
refused.Message.ShouldContain("budget"); refused.Message.ShouldContain("budget");
target.Server.Connections.ShouldBe(1); target.Server.Connections.ShouldBe(1);
target.ForwardOpens.ShouldBe(1); target.ForwardOpens.ShouldBe(1);
} conn.Diagnostics.OpenAttempts.ShouldBe(2); // the refused one is counted too
finally
{
Volatile.Write(ref CipConnection.OpenAttemptBudget, -1);
} }
} }
[Fact]
public async Task An_open_attempt_limit_is_scoped_to_one_device_and_lifted_by_its_scope()
{
// Gitea #566 / #581: the limit is per DeviceKey on one pool's diagnostics, not a process-wide static, so
// limiting the press under test can never refuse an open to any other device.
await using var limited = new FakeCipTarget();
await using var other = new FakeCipTarget();
var time = new ManualTimeProvider();
var pool = new DeviceSessionPool(time);
var limitedKey = Key(limited);
using (pool.Diagnostics.LimitOpenAttempts(limitedKey, 0))
{
await using var otherLease = pool.Acquire(Key(other), Options());
(await otherLease.Scheduler.SendAsync(ReadTag, TestCt)).GeneralStatus.ShouldBe((byte)0);
await using var limitedLease = pool.Acquire(limitedKey, Options());
var refused = await Should.ThrowAsync<AbTransportException>(() => limitedLease.Scheduler.SendAsync(ReadTag, TestCt));
refused.Status.ShouldBe(AbStatus.Abort);
refused.Message.ShouldContain("budget");
Should.Throw<InvalidOperationException>(() => pool.Diagnostics.LimitOpenAttempts(limitedKey, 5)); // one at a time
}
pool.Diagnostics.HasOpenAttemptLimit(limitedKey).ShouldBeFalse();
time.Advance(TimeSpan.FromSeconds(5)); // the refused attempt still started the device's throttle interval
await using var lifted = pool.Acquire(limitedKey, Options());
(await lifted.Scheduler.SendAsync(ReadTag, TestCt).WaitAsync(Guard, TestCt)).GeneralStatus.ShouldBe((byte)0);
limited.ForwardOpens.ShouldBe(1); // the refused attempt never reached the wire; the lifted one did
other.ForwardOpens.ShouldBe(1);
}
private static DeviceKey Key(FakeCipTarget target) => new("127.0.0.1", target.Port, "1,0", "ControlLogix");
[Fact] [Fact]
public async Task Connection_in_use_does_not_fall_back() public async Task Connection_in_use_does_not_fall_back()
{ {
@@ -245,7 +276,7 @@ public sealed class CipConnectionTests
}; };
var time = new ManualTimeProvider(); var time = new ManualTimeProvider();
await using var conn = Create(target, Options(), time); await using var conn = Create(target, Options(), time);
var before = Volatile.Read(ref CipConnection.ForwardOpenCount); var before = conn.Diagnostics.ForwardOpens;
using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestCt); using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestCt);
var pending = conn.SendAsync(ReadTag, cts.Token); var pending = conn.SendAsync(ReadTag, cts.Token);
@@ -256,7 +287,7 @@ public sealed class CipConnectionTests
await conn.SendAsync(ReadTag, TestCt); await conn.SendAsync(ReadTag, TestCt);
(Volatile.Read(ref CipConnection.ForwardOpenCount) - before).ShouldBe(1); (conn.Diagnostics.ForwardOpens - before).ShouldBe(1);
target.ForwardOpens.ShouldBe(1); target.ForwardOpens.ShouldBe(1);
target.Server.Connections.ShouldBe(1); target.Server.Connections.ShouldBe(1);
target.ConnectedRequests.ShouldBe(2); target.ConnectedRequests.ShouldBe(2);
@@ -321,7 +352,7 @@ public sealed class CipConnectionTests
await using var target = new FakeCipTarget(); await using var target = new FakeCipTarget();
var time = new ManualTimeProvider(); var time = new ManualTimeProvider();
await using var conn = Create(target, Options(), time); await using var conn = Create(target, Options(), time);
var before = Volatile.Read(ref CipConnection.RequestsSent); var before = conn.Diagnostics.RequestsSent;
await conn.SendAsync(ReadTag, TestCt); await conn.SendAsync(ReadTag, TestCt);
time.Advance(TimeSpan.FromSeconds(8)); time.Advance(TimeSpan.FromSeconds(8));
@@ -336,7 +367,7 @@ public sealed class CipConnectionTests
time.Advance(TimeSpan.FromSeconds(1)); time.Advance(TimeSpan.FromSeconds(1));
conn.KeepAlivesSent.ShouldBe(2); conn.KeepAlivesSent.ShouldBe(2);
await FakeCipTarget.Until(() => target.ConnectedRequests == 3, Guard, "the second keep-alive"); await FakeCipTarget.Until(() => target.ConnectedRequests == 3, Guard, "the second keep-alive");
(Volatile.Read(ref CipConnection.RequestsSent) - before).ShouldBe(3); // keep-alives count (conn.Diagnostics.RequestsSent - before).ShouldBe(3); // keep-alives count
target.ForwardOpens.ShouldBe(1); target.ForwardOpens.ShouldBe(1);
} }
@@ -411,4 +442,159 @@ public sealed class CipConnectionTests
await dispose.WaitAsync(Guard, TestCt); await dispose.WaitAsync(Guard, TestCt);
target.ForwardOpens.ShouldBe(1); // nothing is opened once disposal has begun target.ForwardOpens.ShouldBe(1); // nothing is opened once disposal has begun
} }
[Fact]
public async Task Disposing_during_the_session_registration_never_puts_a_Forward_Open_on_the_wire()
{
// Gitea #566: disposal that begins while the TCP connect / RegisterSession is on the wire lets that step
// finish (it is bounded), then releases the session instead of sending a Forward Open nobody would close.
await using var target = new FakeCipTarget();
var registerHeld = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
target.Server.RegisterSessionGate = () => registerHeld.Task;
var time = new ManualTimeProvider();
var conn = Create(target, Options(), time);
var send = conn.SendAsync(ReadTag, TestCt);
await target.Server.WaitForFrameAsync(f => FakeEipServer.Command(f) == EncapCommand.RegisterSession, Guard);
var dispose = conn.DisposeAsync().AsTask();
conn.OpenInProgress.ShouldBeTrue(); // disposal is waiting out the attempt on the wire...
dispose.IsCompleted.ShouldBeFalse(); // ...rather than abandoning it
registerHeld.SetResult();
(await Should.ThrowAsync<AbTransportException>(() => send.WaitAsync(Guard, TestCt))).Status.ShouldBe(AbStatus.Abort);
await dispose.WaitAsync(Guard, TestCt);
// The session is released (positive control); the server records frames in arrival order, so a Forward
// Open sent before the UnRegisterSession would already be counted.
await target.Server.WaitForFrameAsync(f => FakeEipServer.Command(f) == EncapCommand.UnRegisterSession, Guard);
target.ForwardOpens.ShouldBe(0);
target.ForwardCloses.ShouldBe(0);
conn.Diagnostics.OpenAttempts.ShouldBe(1);
}
private const ushort AbandonedOpenRefusal = 0x7A11; // an extended status no other test uses
[Fact]
public async Task An_open_every_waiter_abandoned_leaves_no_unobserved_task_exception()
{
// Gitea #566: the shared open fails AFTER its only waiter gave up (its own token). Nothing awaits the
// open's task then; without the observer it would surface as UnobservedTaskException at finalization.
var unobserved = await UnobservedTaskProbe.CollectAsync(AbandonAFailingOpenAsync, Guard);
unobserved.ShouldNotContain(e => e.Message.Contains($"0x{AbandonedOpenRefusal:X4}"));
}
// Kept out of the test body so no local in the test's own state machine roots the abandoned open.
[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
private static async Task AbandonAFailingOpenAsync()
{
await using var target = new FakeCipTarget { ForwardOpenReply = FakeCipTarget.Refuse(0x01, AbandonedOpenRefusal) };
var openHeld = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
target.ForwardOpenGate = () => openHeld.Task;
var time = new ManualTimeProvider();
await using var conn = Create(target, Options(size: 504), time);
using (var cts = new CancellationTokenSource())
{
var waiter = conn.SendAsync(ReadTag, cts.Token);
await FakeCipTarget.Until(() => target.ForwardOpens == 1, Guard, "the Forward Open");
await cts.CancelAsync();
await Should.ThrowAsync<OperationCanceledException>(() => waiter);
}
openHeld.SetResult(); // the target refuses, after every waiter has gone
await FakeCipTarget.Until(() => !conn.OpenInProgress, Guard, "the abandoned open to fail");
}
// ---- CIP-level close (Gitea #566) ------------------------------------------------------------
private static byte[] Ok(byte[] cip) => [(byte)(cip[0] | 0x80), 0, 0, 0];
private static byte[] Status(byte[] cip, byte general, ushort extended) =>
[(byte)(cip[0] | 0x80), 0, general, 1, (byte)extended, (byte)(extended >> 8)];
[Theory]
[InlineData(ConnectionManagerServices.TargetConnectionNotFound)]
[InlineData(ConnectionManagerServices.ConnectionTimedOut)]
public async Task A_reply_saying_the_CIP_connection_is_gone_retires_it_and_the_next_send_reopens(ushort extended)
{
// The target dropped the class-3 connection but the TCP session is still up: every later connected request
// would get the same answer with the session looking healthy, so the connection is recycled at once.
await using var target = new FakeCipTarget();
var gone = 0;
target.ConnectedHandler = cip => Task.FromResult<byte[]?>(
Interlocked.Exchange(ref gone, 0) == 1 ? Status(cip, 0x01, extended) : Ok(cip));
var time = new ManualTimeProvider();
await using var conn = Create(target, Options(), time);
await conn.SendAsync(ReadTag, TestCt);
Volatile.Write(ref gone, 1);
var ex = await Should.ThrowAsync<AbTransportException>(() => conn.SendAsync(ReadTag, TestCt));
ex.Status.ShouldBe(AbStatus.BadConnection);
ex.Message.ShouldContain($"0x{extended:X4}");
conn.Diagnostics.ConnectionsClosedByTarget.ShouldBe(1);
await target.Server.WaitForFrameAsync(f => FakeEipServer.Command(f) == EncapCommand.UnRegisterSession, Guard);
await FakeCipTarget.Until(() => target.Server.OpenConnections == 0, Guard, "the session to close");
target.ForwardCloses.ShouldBe(0); // the target just said there is nothing to close
var reopen = conn.SendAsync(ReadTag, TestCt);
await time.WaitForTimerDueAtAsync(time.Start + TimeSpan.FromSeconds(1), Guard); // through the throttle
time.Advance(TimeSpan.FromSeconds(1));
CipReply.Parse(await reopen.WaitAsync(Guard, TestCt)).GeneralStatus.ShouldBe((byte)0);
target.ForwardOpens.ShouldBe(2);
target.Server.Connections.ShouldBe(2);
}
[Fact]
public async Task A_keepalive_told_the_connection_is_gone_recycles_it_before_any_request_fails()
{
await using var target = new FakeCipTarget();
target.ConnectedHandler = cip => Task.FromResult<byte[]?>(
cip.SequenceEqual(CipConnection.KeepAliveRequest) ? Status(cip, 0x01, 0x0107) : Ok(cip));
var time = new ManualTimeProvider();
await using var conn = Create(target, Options(), time);
await conn.SendAsync(ReadTag, TestCt);
time.Advance(TimeSpan.FromSeconds(8)); // idle: the keep-alive goes out and is told the connection is gone
await FakeCipTarget.Until(() => conn.Diagnostics.ConnectionsClosedByTarget == 1, Guard, "the retirement");
await FakeCipTarget.Until(() => target.Server.OpenConnections == 0, Guard, "the session to close");
// The next request reopens (the throttle interval is long past) instead of failing on a dead connection.
CipReply.Parse(await conn.SendAsync(ReadTag, TestCt).WaitAsync(Guard, TestCt)).GeneralStatus.ShouldBe((byte)0);
target.ForwardOpens.ShouldBe(2);
}
[Fact]
public async Task Other_error_statuses_describe_the_request_and_leave_the_connection_alone()
{
// Negative control for the recycle: a tag-level error and a connection-failure status that is not about
// this connection come back as ordinary replies, and the next request rides the same connection.
await using var target = new FakeCipTarget();
var replies = new Queue<Func<byte[], byte[]>>(
[
cip => Status(cip, 0x04, 0x0000), // path segment error: the tag
cip => Status(cip, 0x01, 0x0204), // unconnected request timed out: not this connection
Ok,
]);
target.ConnectedHandler = cip => Task.FromResult<byte[]?>(replies.Dequeue()(cip));
var time = new ManualTimeProvider();
await using var conn = Create(target, Options(), time);
CipReply.Parse(await conn.SendAsync(ReadTag, TestCt)).GeneralStatus.ShouldBe((byte)0x04);
CipReply.Parse(await conn.SendAsync(ReadTag, TestCt)).GeneralStatus.ShouldBe((byte)0x01);
CipReply.Parse(await conn.SendAsync(ReadTag, TestCt)).GeneralStatus.ShouldBe((byte)0); // same connection
conn.Diagnostics.ConnectionsClosedByTarget.ShouldBe(0);
target.ForwardOpens.ShouldBe(1);
target.Server.Connections.ShouldBe(1);
}
[Fact]
public void Only_connection_failure_0107_and_0203_mean_the_connection_is_gone()
{
ConnectionManagerServices.IsConnectionGone([0xCC, 0, 0x01, 1, 0x07, 0x01]).ShouldBe((ushort)0x0107);
ConnectionManagerServices.IsConnectionGone([0xCC, 0, 0x01, 1, 0x03, 0x02]).ShouldBe((ushort)0x0203);
ConnectionManagerServices.IsConnectionGone([0xCC, 0, 0x01, 1, 0x04, 0x02]).ShouldBeNull(); // 0x0204
ConnectionManagerServices.IsConnectionGone([0xCC, 0, 0x01, 0]).ShouldBeNull(); // no extended word
ConnectionManagerServices.IsConnectionGone([0xCC, 0, 0x05, 1, 0x07, 0x01]).ShouldBeNull(); // not 0x01
ConnectionManagerServices.IsConnectionGone([0xCC, 0, 0, 0]).ShouldBeNull();
}
} }
@@ -69,7 +69,8 @@ public sealed class ForwardOpenThrottleTests
var first = throttle.RunAsync(Attempt, TestCt); var first = throttle.RunAsync(Attempt, TestCt);
time.Advance(TimeSpan.FromSeconds(5)); // the interval is no obstacle; only the in-flight one is time.Advance(TimeSpan.FromSeconds(5)); // the interval is no obstacle; only the in-flight one is
var others = Enumerable.Range(0, 4).Select(_ => throttle.RunAsync(Attempt, TestCt)).ToArray(); var others = Enumerable.Range(0, 4).Select(_ => throttle.RunAsync(Attempt, TestCt)).ToArray();
await Task.Delay(50, TestCt); // Positive control (#583): all four have reached the throttle and are parked behind the in-flight attempt.
await FakeCipTarget.Until(() => throttle.Queued == 4, Guard, "four callers queued behind the first");
Volatile.Read(ref inFlight).ShouldBe(1); Volatile.Read(ref inFlight).ShouldBe(1);
others.ShouldAllBe(t => !t.IsCompleted); others.ShouldAllBe(t => !t.IsCompleted);
@@ -131,8 +132,8 @@ public sealed class ForwardOpenThrottleTests
await using var target = new FakeCipTarget { ForwardOpenReply = FakeCipTarget.Refuse(0x01, 0x0113) }; await using var target = new FakeCipTarget { ForwardOpenReply = FakeCipTarget.Refuse(0x01, 0x0113) };
var time = new ManualTimeProvider(); var time = new ManualTimeProvider();
var options = new ConnectionOptions { ConnectionSize = 504, Timeout = TimeSpan.FromSeconds(2) }; var options = new ConnectionOptions { ConnectionSize = 504, Timeout = TimeSpan.FromSeconds(2) };
var conn = new CipConnection(new DeviceKey("127.0.0.1", target.Port, "1,0", "ControlLogix"), options, var throttle = new ForwardOpenThrottle(options.MinForwardOpenInterval, time);
new ForwardOpenThrottle(options.MinForwardOpenInterval, time), time); var conn = new CipConnection(new DeviceKey("127.0.0.1", target.Port, "1,0", "ControlLogix"), options, throttle, time);
using var stop = CancellationTokenSource.CreateLinkedTokenSource(TestCt); using var stop = CancellationTokenSource.CreateLinkedTokenSource(TestCt);
var failures = 0; var failures = 0;
@@ -151,7 +152,10 @@ public sealed class ForwardOpenThrottleTests
await Task.Delay(10, TestCt); await Task.Delay(10, TestCt);
time.Advance(TimeSpan.FromMilliseconds(100)); time.Advance(TimeSpan.FromMilliseconds(100));
} }
await Task.Delay(100, TestCt); // let any attempt released by the last step reach the wire // Positive control (#583), instead of a fixed settle: wait until the next attempt is parked on the interval.
// Then no attempt is in flight and none can start until the clock moves, so every attempt the last step
// released has already reached the wire and been counted.
await FakeCipTarget.Until(() => throttle.IsWaitingOutInterval, Guard, "the next attempt to park on the interval");
await stop.CancelAsync(); await stop.CancelAsync();
await Task.WhenAll(callers).WaitAsync(Guard, TestCt); await Task.WhenAll(callers).WaitAsync(Guard, TestCt);
@@ -203,7 +203,9 @@ public sealed class DeviceSessionPoolTests
await using var c = pool.Acquire(key, Options()); await using var c = pool.Acquire(key, Options());
var cSend = c.Scheduler.SendAsync(ReadTag, TestCt); var cSend = c.Scheduler.SendAsync(ReadTag, TestCt);
await Task.Delay(300, TestCt); // Positive control (#583): C's open has reached the predecessor wait and parked there, so the absence of
// its Forward Open below is the wait holding it, not C merely not having got that far yet.
await FakeCipTarget.Until(() => pool.Diagnostics.PredecessorWaits == 1, Guard, "C to park behind A's close");
target.ForwardOpens.ShouldBe(1); // C is held back behind A target.ForwardOpens.ShouldBe(1); // C is held back behind A
cSend.IsCompleted.ShouldBeFalse(); cSend.IsCompleted.ShouldBeFalse();
@@ -292,7 +294,7 @@ public sealed class DeviceSessionPoolTests
time.Advance(TimeSpan.FromSeconds(5)); // the throttle is no obstacle time.Advance(TimeSpan.FromSeconds(5)); // the throttle is no obstacle
await using var b = pool.Acquire(key, Options()); await using var b = pool.Acquire(key, Options());
var bSend = b.Scheduler.SendAsync(ReadTag, TestCt); var bSend = b.Scheduler.SendAsync(ReadTag, TestCt);
await Task.Delay(300, TestCt); await FakeCipTarget.Until(() => pool.Diagnostics.PredecessorWaits == 1, Guard, "B to park behind A's close");
target.ForwardOpens.ShouldBe(1); // held behind A's close target.ForwardOpens.ShouldBe(1); // held behind A's close
bSend.IsCompleted.ShouldBeFalse(); bSend.IsCompleted.ShouldBeFalse();
@@ -304,6 +306,72 @@ public sealed class DeviceSessionPoolTests
pool.EntryCount.ShouldBe(1); pool.EntryCount.ShouldBe(1);
} }
/// <summary>
/// Gitea #566: releasing a lease while the device's shared open is still on the wire does not wait for that
/// open (up to its whole open budget) — <see cref="ConnectionLease.ReleaseInBackground"/> (#579) returns at
/// once, and the teardown it hands back still Forward Closes what the open produced once it lands.
/// </summary>
[Fact]
public async Task Release_in_background_does_not_wait_behind_an_in_flight_shared_open()
{
await using var target = new FakeCipTarget();
var openHeld = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
target.ForwardOpenGate = () => openHeld.Task;
var pool = new DeviceSessionPool(new ManualTimeProvider());
var key = new DeviceKey("127.0.0.1", target.Port, "1,0", "ControlLogix");
var lease = pool.Acquire(key, Options());
var send = lease.Scheduler.SendAsync(ReadTag, TestCt);
await FakeCipTarget.Until(() => target.ForwardOpens == 1, Guard, "the Forward Open");
((CipConnection)lease.Channel).OpenInProgress.ShouldBeTrue();
var teardown = lease.ReleaseInBackground(); // returns synchronously: the open is still held
pool.EntryCount.ShouldBe(0);
teardown.IsCompleted.ShouldBeFalse("the teardown waits for the open on the wire; the release call did not");
(await Should.ThrowAsync<AbTransportException>(() => send.WaitAsync(Guard, TestCt))).Status.ShouldBe(AbStatus.BadConnection);
openHeld.SetResult();
await teardown.WaitAsync(Guard, TestCt);
target.ForwardCloses.ShouldBe(1); // what the late-accepted open created is still closed
}
/// <summary>Gitea #566: the close path disposes the connection even when the scheduler's dispose throws —
/// otherwise the connection's keep-alive would hold the press connection open forever.</summary>
[Fact]
public async Task Close_disposes_the_connection_even_when_the_scheduler_dispose_throws()
{
var logger = new CapturingLogger();
var scheduler = new Disposable(() => throw new InvalidOperationException("scheduler dispose failed"));
var connection = new Disposable(() => { });
var key = new DeviceKey("plc1", 44818, "1,0", "ControlLogix");
await DeviceSessionPool.CloseAsync(key, scheduler, connection, logger);
scheduler.Disposed.ShouldBe(1);
connection.Disposed.ShouldBe(1);
logger.Warnings.ShouldBe(1);
// And a throwing connection dispose is logged, never propagated to the release.
var failing = new Disposable(() => throw new InvalidOperationException("close failed"));
await DeviceSessionPool.CloseAsync(key, new Disposable(() => { }), failing, logger);
failing.Disposed.ShouldBe(1);
logger.Warnings.ShouldBe(2);
}
private sealed class Disposable(Action onDispose) : IAsyncDisposable
{
private int _disposed;
public int Disposed => Volatile.Read(ref _disposed);
public ValueTask DisposeAsync()
{
Interlocked.Increment(ref _disposed);
onDispose();
return ValueTask.CompletedTask;
}
}
[Fact] [Fact]
public void Shared_is_one_process_wide_instance() => public void Shared_is_one_process_wide_instance() =>
DeviceSessionPool.Shared.ShouldBeSameAs(DeviceSessionPool.Shared); DeviceSessionPool.Shared.ShouldBeSameAs(DeviceSessionPool.Shared);
@@ -271,28 +271,10 @@ public sealed class EncapsulationClientTests
[Fact] [Fact]
public async Task An_abandoned_exchange_that_times_out_leaves_no_unobserved_task_exception() public async Task An_abandoned_exchange_that_times_out_leaves_no_unobserved_task_exception()
{ {
var unobserved = new List<Exception>(); // Gitea #583: collected against a positive control (a deliberately unobserved fault that MUST be reported),
EventHandler<UnobservedTaskExceptionEventArgs> onUnobserved = (_, e) => // not after a fixed number of GC cycles.
{ var unobserved = await UnobservedTaskProbe.CollectAsync(AbandonAnExchangeThatTimesOut, Guard);
if (e.Exception.Flatten().InnerExceptions.Any(x => x.Message.Contains("within 317 ms"))) unobserved.ShouldNotContain(x => x.Message.Contains("within 317 ms"));
lock (unobserved) unobserved.Add(e.Exception);
};
TaskScheduler.UnobservedTaskException += onUnobserved;
try
{
await AbandonAnExchangeThatTimesOut();
for (var i = 0; i < 5; i++)
{
GC.Collect();
GC.WaitForPendingFinalizers();
await Task.Delay(20, TestCt);
}
lock (unobserved) unobserved.ShouldBeEmpty();
}
finally
{
TaskScheduler.UnobservedTaskException -= onUnobserved;
}
} }
// Kept out of the test body so no local in the test's own state machine roots the abandoned task. // Kept out of the test body so no local in the test's own state machine roots the abandoned task.
@@ -61,6 +61,9 @@ internal sealed class FakeEipServer : IAsyncDisposable
/// <summary>Synchronous convenience setter for <see cref="HandlerAsync"/>.</summary> /// <summary>Synchronous convenience setter for <see cref="HandlerAsync"/>.</summary>
public Func<byte[], byte[]?> Handler { set => HandlerAsync = req => Task.FromResult(value(req)); } public Func<byte[], byte[]?> Handler { set => HandlerAsync = req => Task.FromResult(value(req)); }
/// <summary>Awaited before a RegisterSession is answered (the request is already recorded).</summary>
public Func<Task>? RegisterSessionGate { get; set; }
/// <summary>Never answer non-session requests.</summary> /// <summary>Never answer non-session requests.</summary>
public bool Stall { get; set; } public bool Stall { get; set; }
@@ -160,6 +163,7 @@ internal sealed class FakeEipServer : IAsyncDisposable
switch (Command(frame)) switch (Command(frame))
{ {
case EncapCommand.RegisterSession: case EncapCommand.RegisterSession:
if (RegisterSessionGate is { } gate) await gate();
reply = (byte[])frame.Clone(); reply = (byte[])frame.Clone();
BinaryPrimitives.WriteUInt32LittleEndian(reply.AsSpan(4), Session); BinaryPrimitives.WriteUInt32LittleEndian(reply.AsSpan(4), Session);
break; break;
@@ -0,0 +1,54 @@
using System.Collections.Concurrent;
using System.Runtime.CompilerServices;
namespace ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Tests.Fakes;
/// <summary>
/// Collects <see cref="TaskScheduler.UnobservedTaskException"/>s raised after some code abandons work, with a
/// positive control (Gitea #583): after the abandon, a task is deliberately faulted and dropped, and garbage is
/// collected until THAT task's exception has been reported. Only then is the collection returned — so "nothing
/// of ours was reported" means the finalizer pass that would have reported it really ran, not merely that a
/// fixed number of GC cycles went by before the runtime got round to it.
/// </summary>
internal static class UnobservedTaskProbe
{
public static async Task<IReadOnlyList<Exception>> CollectAsync(Func<Task> abandon, TimeSpan timeout)
{
var marker = $"unobserved-task-probe-control-{Guid.NewGuid():N}";
var seen = new ConcurrentQueue<Exception>();
var controlReported = 0;
EventHandler<UnobservedTaskExceptionEventArgs> handler = (_, e) =>
{
foreach (var ex in e.Exception.Flatten().InnerExceptions)
{
if (ex.Message == marker) Volatile.Write(ref controlReported, 1);
else seen.Enqueue(ex);
}
};
TaskScheduler.UnobservedTaskException += handler;
try
{
await abandon();
AbandonControl(marker);
var giveUp = DateTime.UtcNow + timeout;
while (Volatile.Read(ref controlReported) == 0)
{
if (DateTime.UtcNow > giveUp)
throw new TimeoutException("The positive control's unobserved exception was never reported.");
GC.Collect();
GC.WaitForPendingFinalizers();
await Task.Delay(10);
}
// One more full pass, so anything that became garbage in the same cycle as the control is reported too.
GC.Collect();
GC.WaitForPendingFinalizers();
}
finally
{
TaskScheduler.UnobservedTaskException -= handler;
}
return [.. seen];
}
[MethodImpl(MethodImplOptions.NoInlining)]
private static void AbandonControl(string marker) => _ = Task.FromException(new InvalidOperationException(marker));
}
@@ -92,9 +92,9 @@ internal sealed class DifferentialAbortedException(string message, Exception? in
/// <para><b>Stop on first failure (review r1 C1).</b> The first connection-level failure on either side — /// <para><b>Stop on first failure (review r1 C1).</b> The first connection-level failure on either side —
/// connect / RegisterSession / Forward Open refused, timeout, session lost — aborts the whole run with the /// connect / RegisterSession / Forward Open refused, timeout, session lost — aborts the whole run with the
/// operation and status; nothing is retried. The native side is additionally capped at ONE open attempt /// operation and status; nothing is retried. The native side is additionally capped at ONE open attempt
/// process-wide (<see cref="CipConnection.OpenAttemptBudget"/>), so a reconnect cannot reach the wire even /// to the press under test (<see cref="ConnectionDiagnostics.LimitOpenAttempts"/>), so a reconnect cannot reach the wire even
/// from inside an enumerator that swallows a per-program failure, and the run aborts the moment /// from inside an enumerator that swallows a per-program failure, and the run aborts the moment
/// <see cref="CipConnection.ForwardOpenCount"/> passes 1.</para> /// <see cref="ConnectionDiagnostics.ForwardOpens"/> passes 1.</para>
/// </summary> /// </summary>
internal static class DifferentialRun internal static class DifferentialRun
{ {
@@ -222,9 +222,9 @@ internal static class DifferentialRun
/// <summary> /// <summary>
/// Runs <paramref name="body"/> with every native operation sharing ONE connection, held open by an anchor /// Runs <paramref name="body"/> with every native operation sharing ONE connection, held open by an anchor
/// lease: the process-wide Forward Open counter is reset, the open-attempt budget is set to exactly one /// lease: the shared pool's Forward Open counters are reset, the press is limited to exactly one
/// more attempt, and <paramref name="body"/> receives a check to call after each operation that aborts the /// open attempt, and <paramref name="body"/> receives a check to call after each operation that aborts the
/// moment a second Forward Open reaches the wire. The anchor is released and the budget restored in /// moment a second Forward Open reaches the wire. The anchor is released and the limit lifted in
/// <c>finally</c>. Shared by the differential and the latency comparison. /// <c>finally</c>. Shared by the differential and the latency comparison.
/// </summary> /// </summary>
/// <returns>The body's result, the Forward Opens counted between the reset and the anchor release, and the /// <returns>The body's result, the Forward Opens counted between the reset and the anchor release, and the
@@ -233,43 +233,38 @@ internal static class DifferentialRun
string phase, DifferentialTarget target, Func<Action<string>, Task<T>> body) string phase, DifferentialTarget target, Func<Action<string>, Task<T>> body)
{ {
var anchorParams = target.Params("@tags"); var anchorParams = target.Params("@tags");
var key = NativeAbCipDevice.Key(anchorParams);
var diagnostics = DeviceSessionPool.Shared.Diagnostics;
void CheckOpens(string operation) void CheckOpens(string operation)
{ {
var opens = Volatile.Read(ref CipConnection.ForwardOpenCount); var opens = diagnostics.ForwardOpens;
if (opens > 1) if (opens > 1)
throw new DifferentialAbortedException( throw new DifferentialAbortedException(
$"Differential ABORTED in the {phase} phase after {operation}: {opens} Forward Open frames on the wire, " + $"Differential ABORTED in the {phase} phase after {operation}: {opens} Forward Open frames on the wire, " +
"expected exactly one (a reconnect, or a Large→regular fallback). Nothing was retried."); "expected exactly one (a reconnect, or a Large→regular fallback). Nothing was retried.");
} }
Interlocked.Exchange(ref CipConnection.ForwardOpenCount, 0); diagnostics.ResetForwardCounts();
Interlocked.Exchange(ref CipConnection.ForwardClosesSent, 0);
Interlocked.Exchange(ref CipConnection.ForwardClosesAcked, 0);
Volatile.Write(ref CipConnection.OpenAttemptBudget, Volatile.Read(ref CipConnection.OpenAttempts) + 1);
T result; T result;
int forwardOpens; int forwardOpens;
NativeCloses forwardCloses; NativeCloses forwardCloses;
try // Exactly one open attempt to the press under test; lifted when the scope ends, whatever happens.
using (diagnostics.LimitOpenAttempts(key, 1))
{ {
// Acquire does no I/O; the key and options carry no tag-specific field, so every lease shares this entry. // Acquire does no I/O; the key and options carry no tag-specific field, so every lease shares this entry.
var anchor = DeviceSessionPool.Shared.Acquire(NativeAbCipDevice.Key(anchorParams), NativeAbCipDevice.Options(anchorParams)); var anchor = DeviceSessionPool.Shared.Acquire(key, NativeAbCipDevice.Options(anchorParams));
try try
{ {
result = await body(CheckOpens).ConfigureAwait(false); result = await body(CheckOpens).ConfigureAwait(false);
} }
finally finally
{ {
forwardOpens = Volatile.Read(ref CipConnection.ForwardOpenCount); forwardOpens = diagnostics.ForwardOpens;
await anchor.DisposeAsync().ConfigureAwait(false); await anchor.DisposeAsync().ConfigureAwait(false);
forwardCloses = new NativeCloses( forwardCloses = new NativeCloses(diagnostics.ForwardClosesSent, diagnostics.ForwardClosesAcked);
Volatile.Read(ref CipConnection.ForwardClosesSent), Volatile.Read(ref CipConnection.ForwardClosesAcked));
} }
} }
finally
{
Volatile.Write(ref CipConnection.OpenAttemptBudget, -1);
}
return (result, forwardOpens, forwardCloses); return (result, forwardOpens, forwardCloses);
} }
@@ -30,12 +30,28 @@ public sealed class DifferentialSafetyTests
() => DifferentialRun.RunNativePhaseAsync(target, selection, TestContext.Current.CancellationToken)); () => DifferentialRun.RunNativePhaseAsync(target, selection, TestContext.Current.CancellationToken));
ex.Message.ShouldContain("native phase at browse"); ex.Message.ShouldContain("native phase at browse");
// Long enough for a throttled (≥ 1 s) reconnect to have reached the listener, had one been allowed. DeviceSessionPool.Shared.Diagnostics.HasOpenAttemptLimit(Key(target)).ShouldBeFalse("the limit is lifted after the phase");
await Task.Delay(TimeSpan.FromSeconds(2.5), TestContext.Current.CancellationToken); await ControlOpenAsync(target);
endpoint.Connections.ShouldBe(1); endpoint.Connections.ShouldBe(2, "the phase's one connect, then the control's — no reconnect from the phase");
Volatile.Read(ref CipConnection.OpenAttemptBudget).ShouldBe(-1, "the budget is restored after the phase");
} }
/// <summary>
/// Positive control (Gitea #583) for "no reconnect reached the listener": one more open to the same device,
/// made after the phase, once its limit is lifted. It goes through the same per-device Forward Open throttle,
/// whose waiters are served in arrival order, so by the time it has reached the listener (it is counted on
/// accept, before the hang-up that fails it) any reconnect the phase had queued would already have got there
/// — and, the limit being gone, nothing would have refused it.
/// </summary>
private static async Task ControlOpenAsync(DifferentialTarget target)
{
var p = target.Params("@tags");
await using var lease = DeviceSessionPool.Shared.Acquire(NativeAbCipDevice.Key(p), NativeAbCipDevice.Options(p));
await Should.ThrowAsync<AbTransportException>(
() => lease.Scheduler.SendAsync([0x4C, 0x00], TestContext.Current.CancellationToken));
}
private static DeviceKey Key(DifferentialTarget target) => NativeAbCipDevice.Key(target.Params("@tags"));
[Fact] [Fact]
public async Task A_failing_endpoint_aborts_the_libplctag_phase_before_any_tag_is_read() public async Task A_failing_endpoint_aborts_the_libplctag_phase_before_any_tag_is_read()
{ {
@@ -146,13 +162,13 @@ public sealed class DifferentialSafetyTests
public async Task The_open_attempt_budget_alone_stops_a_second_open_even_when_nothing_aborts_the_phase() public async Task The_open_attempt_budget_alone_stops_a_second_open_even_when_nothing_aborts_the_phase()
{ {
// Unlike the abort test above, the body here SWALLOWS every failure and keeps going, so the harness's // Unlike the abort test above, the body here SWALLOWS every failure and keeps going, so the harness's
// abort-on-first-failure can never end the phase: only CipConnection.OpenAttemptBudget stands between // abort-on-first-failure can never end the phase: only the per-device open-attempt limit stands between
// the second request and the wire. // the second request and the wire.
await using var endpoint = HangUpListener.Start(); await using var endpoint = HangUpListener.Start();
var target = Target(endpoint.Port); var target = Target(endpoint.Port);
var statuses = new List<AbStatus>(); var statuses = new List<AbStatus>();
string? secondMessage = null; string? secondMessage = null;
var attemptsBefore = Volatile.Read(ref CipConnection.OpenAttempts); var attemptsBefore = DeviceSessionPool.Shared.Diagnostics.OpenAttempts;
var tags = AbCipTransportFactories.For(AbCipTransport.Native).Tags; var tags = AbCipTransportFactories.For(AbCipTransport.Native).Tags;
await DifferentialRun.WithNativeAnchorAsync("native", target, async _ => await DifferentialRun.WithNativeAnchorAsync("native", target, async _ =>
@@ -178,11 +194,12 @@ public sealed class DifferentialSafetyTests
statuses.Count.ShouldBe(2, "both requests failed; neither aborted the phase"); statuses.Count.ShouldBe(2, "both requests failed; neither aborted the phase");
statuses[1].ShouldBe(AbStatus.Abort); statuses[1].ShouldBe(AbStatus.Abort);
secondMessage.ShouldNotBeNull().ShouldContain("budget"); secondMessage.ShouldNotBeNull().ShouldContain("budget");
Volatile.Read(ref CipConnection.OpenAttempts).ShouldBe(attemptsBefore + 2, "the second attempt was counted, then refused"); // The pool is process-wide, so another test's opens could move this counter; at least these two are in it.
// Long enough for a throttled reconnect to have reached the listener had the budget let one through. (DeviceSessionPool.Shared.Diagnostics.OpenAttempts - attemptsBefore).ShouldBeGreaterThanOrEqualTo(2,
await Task.Delay(TimeSpan.FromSeconds(1.5), TestContext.Current.CancellationToken); "the second attempt was counted, then refused");
endpoint.Connections.ShouldBe(1, "exactly the one budgeted open attempt reached the wire"); DeviceSessionPool.Shared.Diagnostics.HasOpenAttemptLimit(Key(target)).ShouldBeFalse("the limit is lifted afterwards");
Volatile.Read(ref CipConnection.OpenAttemptBudget).ShouldBe(-1, "the budget is restored afterwards"); await ControlOpenAsync(target);
endpoint.Connections.ShouldBe(2, "exactly the one budgeted open attempt reached the wire, then the control's");
} }
[Fact] [Fact]
@@ -126,7 +126,7 @@ public sealed class PollLatencyComparisonTests(DifferentialRunFixture fixture)
$"{passes.Sum(p => p.TagFailures)} |"; $"{passes.Sum(p => p.TagFailures)} |";
} }
private static string Rtt(ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip.CipConnection.ExchangeStats? s) => private static string Rtt(ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip.ConnectionDiagnostics.ExchangeStats? s) =>
s is not { Count: > 0 } x ? "-" : $"{Ms(x.MinMs)}/{Ms(x.MeanMs)}/{Ms(x.MaxMs)}"; s is not { Count: > 0 } x ? "-" : $"{Ms(x.MinMs)}/{Ms(x.MeanMs)}/{Ms(x.MaxMs)}";
private static string Ms(double ms) => ms.ToString("0.0", CultureInfo.InvariantCulture); private static string Ms(double ms) => ms.ToString("0.0", CultureInfo.InvariantCulture);
@@ -5,13 +5,13 @@ using ZB.MOM.WW.OtOpcUa.Driver.Ab.Transport.Cip;
namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.Differential; namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests.Differential;
/// <summary>One timed burst: every tag read once, concurrently.</summary> /// <summary>One timed burst: every tag read once, concurrently.</summary>
/// <param name="Requests">Native only: <see cref="CipConnection.RequestsSent"/> delta over the pass (keep-alives included).</param> /// <param name="Requests">Native only: <see cref="ConnectionDiagnostics.RequestsSent"/> delta over the pass (keep-alives included).</param>
/// <param name="TagFailures">Reads that ended with a tag-level status (a connection-level one aborts instead).</param> /// <param name="TagFailures">Reads that ended with a tag-level status (a connection-level one aborts instead).</param>
/// <param name="Gc">Collections that ran during the pass, per generation (T21 review r1 M8: a client-side stall shows here).</param> /// <param name="Gc">Collections that ran during the pass, per generation (T21 review r1 M8: a client-side stall shows here).</param>
/// <param name="Rtt">Native only: round trip of each CIP exchange in the pass, from <see cref="CipConnection"/> (a slow /// <param name="Rtt">Native only: round trip of each CIP exchange in the pass, from <see cref="CipConnection"/> (a slow
/// controller shows here; a slow pass with fast exchanges points at the client).</param> /// controller shows here; a slow pass with fast exchanges points at the client).</param>
internal sealed record LatencyPass( internal sealed record LatencyPass(
int Index, TimeSpan Wall, int? Requests, int TagFailures, GcDelta Gc = default, CipConnection.ExchangeStats? Rtt = null); int Index, TimeSpan Wall, int? Requests, int TagFailures, GcDelta Gc = default, ConnectionDiagnostics.ExchangeStats? Rtt = null);
/// <summary>GC collections per generation over a window (gen0 counts include gen1/gen2 collections, as the runtime reports).</summary> /// <summary>GC collections per generation over a window (gen0 counts include gen1/gen2 collections, as the runtime reports).</summary>
internal readonly record struct GcDelta(int Gen0, int Gen1, int Gen2) internal readonly record struct GcDelta(int Gen0, int Gen1, int Gen2)
@@ -137,7 +137,7 @@ internal static class PollLatencyRun
var runtimes = new List<(SelectedTag Tag, IAbCipTagRuntime Runtime)>(); var runtimes = new List<(SelectedTag Tag, IAbCipTagRuntime Runtime)>();
try try
{ {
var before = Volatile.Read(ref CipConnection.RequestsSent); var before = DeviceSessionPool.Shared.Diagnostics.RequestsSent;
foreach (var tag in tags) foreach (var tag in tags)
{ {
var rt = await CreateAsync("latency native", target, nativeFactory, tag, ct).ConfigureAwait(false); var rt = await CreateAsync("latency native", target, nativeFactory, tag, ct).ConfigureAwait(false);
@@ -145,7 +145,7 @@ internal static class PollLatencyRun
if (rt is null) { problems.Add($"native could not read '{tag.Name}', which libplctag read"); continue; } if (rt is null) { problems.Add($"native could not read '{tag.Name}', which libplctag read"); continue; }
runtimes.Add((tag, rt)); runtimes.Add((tag, rt));
} }
var setup = Volatile.Read(ref CipConnection.RequestsSent) - before; var setup = DeviceSessionPool.Shared.Diagnostics.RequestsSent - before;
var passes = await RunPassesAsync("latency native", target, runtimes, countRequests: true, checkOpens, ct) var passes = await RunPassesAsync("latency native", target, runtimes, countRequests: true, checkOpens, ct)
.ConfigureAwait(false); .ConfigureAwait(false);
return (passes, setup); return (passes, setup);
@@ -214,15 +214,15 @@ internal static class PollLatencyRun
{ {
if (p > 1 && target.LatencyPassGapMs > 0) await Task.Delay(target.LatencyPassGapMs, ct).ConfigureAwait(false); if (p > 1 && target.LatencyPassGapMs > 0) await Task.Delay(target.LatencyPassGapMs, ct).ConfigureAwait(false);
var requestsBefore = Volatile.Read(ref CipConnection.RequestsSent); var requestsBefore = DeviceSessionPool.Shared.Diagnostics.RequestsSent;
CipConnection.ResetExchangeStats(); DeviceSessionPool.Shared.Diagnostics.ResetExchangeStats();
var gcBefore = GcDelta.Now(); var gcBefore = GcDelta.Now();
var clock = Stopwatch.StartNew(); var clock = Stopwatch.StartNew();
var outcomes = await Task.WhenAll(runtimes.Select(r => ReadOnceAsync(r.Runtime, ct))).ConfigureAwait(false); var outcomes = await Task.WhenAll(runtimes.Select(r => ReadOnceAsync(r.Runtime, ct))).ConfigureAwait(false);
clock.Stop(); clock.Stop();
var gc = GcDelta.Now() - gcBefore; var gc = GcDelta.Now() - gcBefore;
var rtt = countRequests ? CipConnection.ReadExchangeStats() : (CipConnection.ExchangeStats?)null; var rtt = countRequests ? DeviceSessionPool.Shared.Diagnostics.ReadExchangeStats() : (ConnectionDiagnostics.ExchangeStats?)null;
var requests = Volatile.Read(ref CipConnection.RequestsSent) - requestsBefore; var requests = DeviceSessionPool.Shared.Diagnostics.RequestsSent - requestsBefore;
var failures = 0; var failures = 0;
for (var i = 0; i < outcomes.Length; i++) for (var i = 0; i < outcomes.Length; i++)
@@ -18,7 +18,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests;
[CollectionDefinition(Name, DisableParallelization = true)] [CollectionDefinition(Name, DisableParallelization = true)]
public sealed class LibplctagOrphanReleaseCollection public sealed class LibplctagOrphanReleaseCollection
{ {
/// <summary>The release counters and the rooting seam are process-wide: nothing else may run alongside.</summary> /// <summary>The release counters are process-wide: nothing else may run alongside.</summary>
public const string Name = "libplctag orphan release"; public const string Name = "libplctag orphan release";
} }
@@ -90,23 +90,29 @@ public sealed class LibplctagOrphanReleaseTests
[Fact] [Fact]
public async Task Without_a_readable_handle_the_failed_tag_is_rooted_instead() public async Task Without_a_readable_handle_the_failed_tag_is_rooted_instead()
{ {
// Gitea #581: the "unreadable handle" decision is passed in, not flipped through a process-wide switch.
var rooted = LibplctagOrphanRelease.RootedCount; var rooted = LibplctagOrphanRelease.RootedCount;
var released = LibplctagOrphanRelease.Released; var released = LibplctagOrphanRelease.Released;
var rt = new LibplctagTagRuntime(Refused()); var tag = LibplctagTagRuntime.BuildTag(Refused());
LibplctagOrphanRelease.ForceRootingForTest = true;
try try
{ {
await Should.ThrowAsync<AbTransportException>(() => rt.InitializeAsync(CancellationToken.None)); await Should.ThrowAsync<Exception>(() => tag.InitializeAsync(CancellationToken.None));
var handle = LibplctagOrphanRelease.HandleOf(tag);
handle.ShouldBeGreaterThan(0, "plc_tag_create_ex returned a handle before the failure");
LibplctagOrphanRelease.ReleaseIfOrphaned(tag, canReadHandles: false).ShouldBeNull();
LibplctagOrphanRelease.RootedCount.ShouldBe(rooted + 1); LibplctagOrphanRelease.RootedCount.ShouldBe(rooted + 1);
LibplctagOrphanRelease.Released.ShouldBe(released); LibplctagOrphanRelease.Released.ShouldBe(released);
NativeTagExists(handle).ShouldBeTrue("the rooting fallback destroys nothing");
LibplctagOrphanRelease.HandleOf(tag).ShouldBe(handle);
} }
finally finally
{ {
LibplctagOrphanRelease.ForceRootingForTest = false;
// Clean up the native tag this test deliberately leaked (its callback was unregistered by the timeout). // Clean up the native tag this test deliberately leaked (its callback was unregistered by the timeout).
var handle = rt.NativeHandle; var handle = LibplctagOrphanRelease.HandleOf(tag);
if (handle > 0) plctag.plc_tag_destroy(handle); if (handle > 0) plctag.plc_tag_destroy(handle);
rt.Dispose(); tag.Dispose();
} }
} }