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

fix(s7): probe gate contention is no verdict; Test Connect honours timeoutMs (R07:H7)

Browse Source 
- The background probe reported the host Stopped whenever a read pass held
  _gate past the probe deadline: contention is the connection being used,
  not lost. A contended tick now gives no verdict (counted for tests); a
  real probe failure still reports Stopped. A tick that races a teardown no
  longer reopens a connection on an uninitialised driver.
- S7DriverProbe deserialised S7DriverOptions directly, whose Timeout is a
  TimeSpan no config names, so "timeoutMs" was ignored and every handshake
  waited the whole Test Connect budget. The handshake and test read are now
  bounded by timeoutMs, capped by the caller's timeout.

Refs #742
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
2026-10-22 15:33:38 -04:00
parent 1f7ab80b81
commit a0b635f673
4 changed files with 233 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs
+71 -41
View File
@@ -2792,66 +2792,96 @@ public sealed class S7Driver
{
while (!ct.IsCancellationRequested)
{
var success = false;
// Running / Stopped, or null for "no verdict this tick" — which leaves the host state alone.
HostState? verdict = null;
try
{
// Probe via S7.Net's low-cost GetCpuStatus — returns the CPU state (Run/Stop)
// and is intentionally light on the comms mailbox. Single-word Plc.ReadAsync
// would also work but GetCpuStatus doubles as a "PLC actually up" check.
using var gateCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
gateCts.CancelAfter(_options.Probe.Timeout);
await _gate.WaitAsync(gateCts.Token).ConfigureAwait(false);
try
//
// Gate CONTENTION is not a verdict (R07:H7): a read pass or write batch holding the
// gate past the probe deadline is the connection being USED, and that pass reports
// its own outcome through health. Reporting the host Stopped for it flapped /hosts on
// every long pass against a perfectly healthy PLC.
if (!await _gate.WaitAsync(_options.Probe.Timeout, ct).ConfigureAwait(false))
{
Interlocked.Increment(ref _probeGateContentions);
}
else
{
try
{
// EnsureConnectedAsync doubles as the reconnect backstop: if a read/write
// marked the handle dead (or the socket dropped between ticks), the probe
// reopens it here so recovery doesn't wait for the next data call.
// It runs on the LOOP token, not the (shorter) probe deadline: the connect is bounded by
// _options.Timeout inside, and a timeout there must count as a failure so the
// backoff window opens — cutting it with the probe deadline looked like a caller cancel,
// recorded nothing, and let a black-holed host cost a connect attempt every probe tick.
var plc = await EnsureConnectedAsync(ct).ConfigureAwait(false);
// The probe deadline bounds only the CPU-status round trip.
using var probeCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
probeCts.CancelAfter(_options.Probe.Timeout);
await plc.ReadStatusAsync(probeCts.Token).ConfigureAwait(false);
success = true;
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
// Loop teardown — rethrow to the outer filter (after finally releases the gate).
throw;
}
catch (OperationCanceledException)
{
// Probe deadline fired mid-PDU: the CPU-status response may still be in
// flight, so the handle can't be reused (half-read ⇒ desync). We hold _gate
// here, keeping _plcDead gate-guarded.
_plcDead = true;
}
catch (Exception ex)
{
// Classify + mark while holding _gate so a wire-dead/desynced
// handle whose TcpClient still claims Connected is reopened by the NEXT
// tick's EnsureConnectedAsync instead of being re-probed forever.
MarkConnectionDeadIfFatal(ex);
// A tick that raced a teardown / re-initialize has nothing to probe and must
// not reopen a connection on a driver that is not initialized.
if (_initialized)
verdict = await ProbeOnceAsync(ct).ConfigureAwait(false);
}
finally { _gate.Release(); }
}
finally { _gate.Release(); }
}
catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; }
catch { /* gate-wait timeout / other — treated as Stopped below (gate contention is not a connection fault) */ }
catch { verdict = HostState.Stopped; /* any other probe fault */ }
TransitionTo(success ? HostState.Running : HostState.Stopped);
if (verdict is { } state) TransitionTo(state);
try { await Task.Delay(_options.Probe.Interval, ct).ConfigureAwait(false); }
catch (OperationCanceledException) { return; }
}
}
/// <summary>
/// One probe round trip. Caller holds <see cref="_gate"/>. Returns the verdict; rethrows only
/// loop teardown.
/// </summary>
/// <param name="ct">The probe loop's token.</param>
/// <returns><see cref="HostState.Running"/> or <see cref="HostState.Stopped"/>.</returns>
private async Task<HostState> ProbeOnceAsync(CancellationToken ct)
{
try
{
// EnsureConnectedAsync doubles as the reconnect backstop: if a read/write marked the handle
// dead (or the socket dropped between ticks), the probe reopens it here so recovery doesn't
// wait for the next data call. It runs on the LOOP token, not the (shorter) probe deadline:
// the connect is bounded by _options.Timeout inside, and a timeout there must count as a
// failure so the backoff window opens — cutting it with the probe deadline looked like a
// caller cancel, recorded nothing, and let a black-holed host cost a connect attempt every
// probe tick.
var plc = await EnsureConnectedAsync(ct).ConfigureAwait(false);
// The probe deadline bounds only the CPU-status round trip.
using var probeCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
probeCts.CancelAfter(_options.Probe.Timeout);
await plc.ReadStatusAsync(probeCts.Token).ConfigureAwait(false);
return HostState.Running;
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
throw; // loop teardown — the caller's finally releases the gate
}
catch (OperationCanceledException)
{
// Probe deadline fired mid-PDU: the CPU-status response may still be in flight, so the
// handle can't be reused (half-read ⇒ desync). The caller holds _gate, keeping _plcDead
// gate-guarded.
_plcDead = true;
return HostState.Stopped;
}
catch (Exception ex)
{
// Classify + mark while holding _gate so a wire-dead/desynced handle whose TcpClient still
// claims Connected is reopened by the NEXT tick's EnsureConnectedAsync instead of being
// re-probed forever.
MarkConnectionDeadIfFatal(ex);
return HostState.Stopped;
}
}
/// <summary>Probe ticks that found the gate held past the probe deadline and gave no verdict.</summary>
private int _probeGateContentions;
/// <summary>Count of probe ticks skipped for gate contention. Internal so a test can wait on it.</summary>
internal int ProbeGateContentions => Volatile.Read(ref _probeGateContentions);
private void TransitionTo(HostState newState)
{
HostState old;
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7DriverProbe.cs
+37 -2
View File
@@ -42,6 +42,14 @@ public sealed class S7DriverProbe : IDriverProbe
if (string.IsNullOrWhiteSpace(host) || port <= 0)
return new(false, "Config has no host/port to probe.", null);
// The S7 handshake + test read are bounded by the driver's own per-operation timeout
// (config "timeoutMs", what the deployed driver will wait), capped by the caller's overall
// Test Connect timeout. The options type carries it as a TimeSpan the JSON never names, so it
// used to be ignored and every handshake waited the full Test Connect timeout (R07:H7).
var opTimeout = ReadTimeoutMs(configJson) is { } ms && TimeSpan.FromMilliseconds(ms) < timeout
? TimeSpan.FromMilliseconds(ms)
: timeout;
// Phase 1: bare TCP preflight — fast rejection for unreachable hosts.
var sw = Stopwatch.StartNew();
try
@@ -71,9 +79,9 @@ public sealed class S7DriverProbe : IDriverProbe
var plc = S7Tsap.Resolve(opts) is { } tsap
? new Plc(host, port, tsap)
: new Plc(S7CpuTypeMap.ToS7Net(opts.CpuType), host, port, opts.Rack, opts.Slot);
plc.ReadTimeout = (int)timeout.TotalMilliseconds;
plc.ReadTimeout = (int)opTimeout.TotalMilliseconds;
using var handshakeCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
handshakeCts.CancelAfter(timeout);
handshakeCts.CancelAfter(opTimeout);
try
{
await plc.OpenAsync(handshakeCts.Token);
@@ -148,4 +156,31 @@ public sealed class S7DriverProbe : IDriverProbe
private static (string host, int port) ExtractTarget(S7DriverOptions opts)
=> (opts.Host, opts.Port);
/// <summary>
/// The config's <c>timeoutMs</c> (any casing, as the driver factory binds it), or null when absent
/// or not a positive integer.
/// </summary>
/// <param name="configJson">The driver config.</param>
/// <returns>The configured per-operation timeout in milliseconds, or null.</returns>
internal static int? ReadTimeoutMs(string configJson)
{
try
{
using var doc = JsonDocument.Parse(configJson);
if (doc.RootElement.ValueKind != JsonValueKind.Object) return null;
foreach (var prop in doc.RootElement.EnumerateObject())
{
if (!string.Equals(prop.Name, "timeoutMs", StringComparison.OrdinalIgnoreCase)) continue;
return prop.Value.ValueKind == JsonValueKind.Number && prop.Value.TryGetInt32(out var ms) && ms > 0
? ms
: null;
}
return null;
}
catch (JsonException)
{
return null;
}
}
}