fix(abcip): serialize Read/GetStatus/Decode + Encode/Write/GetStatus per runtime (05/STAB-4)

archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json

@@ -42,7 +42,7 @@
     {
       "id": "B2.3",
       "subject": "B2: wrap ReadSingleAsync/ReadGroupAsync/WriteAsync critical sections in the per-runtime lock; AbCip suite green",
-      "status": "pending",
+      "status": "completed",
       "blockedBy": [
         "B2.2"
       ]

src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs

@@ -542,37 +542,49 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
         try
         {
             var runtime = await EnsureTagRuntimeAsync(device, def, ct).ConfigureAwait(false);
-            await runtime.ReadAsync(ct).ConfigureAwait(false);
 
-            var status = runtime.GetStatus();
+            // Serialise Read → GetStatus → Decode against the shared runtime — the cached libplctag
-            if (status != 0)
+            // Tag handle is not concurrency-safe and GetStatus returns the LAST op's status (05/STAB-4).
+            var opLock = device.GetRuntimeLock(def.Name);
+            await opLock.WaitAsync(ct).ConfigureAwait(false);
+            try
             {
-                // Evict the stale handle so the next call re-creates it.
+                await runtime.ReadAsync(ct).ConfigureAwait(false);
-                // A non-zero status can mean the controller dropped the connection or the tag
-                // handle became permanently invalid (e.g. after a PLC download). Evicting
-                // mirrors the probe loop's recreate-on-failure behaviour.
-                EvictRuntime(device, def.Name);
-                results[fb.OriginalIndex] = new DataValueSnapshot(null,
-                    AbCipStatusMapper.MapLibplctagStatus(status), null, now);
-                _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead,
-                    $"libplctag status {status} reading {reference}");
-                _logger.LogWarning(
-                    "AbCip read returned non-zero libplctag status {LibplctagStatus} for tag {Tag} on device {Device}; " +
-                    "evicting cached runtime so next call re-creates it",
-                    status, reference, def.DeviceHostAddress);
-                return;
-            }
 
-            var tagPath = AbCipTagPath.TryParse(def.TagPath);
+                var status = runtime.GetStatus();
-            var bitIndex = tagPath?.BitIndex;
+                if (status != 0)
-            // Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an
+                {
-            // element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array
+                    // Evict the stale handle so the next call re-creates it.
-            // (ElementCount 1). Scalar tags keep the single-value path.
+                    // A non-zero status can mean the controller dropped the connection or the tag
-            var value = IsArrayTag(def)
+                    // handle became permanently invalid (e.g. after a PLC download). Evicting
-                ? runtime.DecodeArray(def.DataType, Math.Max(1, def.ElementCount))
+                    // mirrors the probe loop's recreate-on-failure behaviour.
-                : runtime.DecodeValue(def.DataType, bitIndex);
+                    EvictRuntime(device, def.Name);
-            results[fb.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now);
+                    results[fb.OriginalIndex] = new DataValueSnapshot(null,
-            _health = new DriverHealth(DriverState.Healthy, now, null);
+                        AbCipStatusMapper.MapLibplctagStatus(status), null, now);
+                    _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead,
+                        $"libplctag status {status} reading {reference}");
+                    _logger.LogWarning(
+                        "AbCip read returned non-zero libplctag status {LibplctagStatus} for tag {Tag} on device {Device}; " +
+                        "evicting cached runtime so next call re-creates it",
+                        status, reference, def.DeviceHostAddress);
+                    return;
+                }
+
+                var tagPath = AbCipTagPath.TryParse(def.TagPath);
+                var bitIndex = tagPath?.BitIndex;
+                // Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an
+                // element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array
+                // (ElementCount 1). Scalar tags keep the single-value path.
+                var value = IsArrayTag(def)
+                    ? runtime.DecodeArray(def.DataType, Math.Max(1, def.ElementCount))
+                    : runtime.DecodeValue(def.DataType, bitIndex);
+                results[fb.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now);
+                _health = new DriverHealth(DriverState.Healthy, now, null);
+            }
+            finally
+            {
+                opLock.Release();
+            }
         }
         catch (OperationCanceledException)
         {
@@ -612,29 +624,41 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
         try
         {
             var runtime = await EnsureTagRuntimeAsync(device, parent, ct).ConfigureAwait(false);
-            await runtime.ReadAsync(ct).ConfigureAwait(false);
 
-            var status = runtime.GetStatus();
+            // Serialise the whole-UDT Read → GetStatus → per-member DecodeValueAt against the shared
-            if (status != 0)
+            // parent runtime — the members decode from that one handle's buffer (05/STAB-4).
+            var opLock = device.GetRuntimeLock(parent.Name);
+            await opLock.WaitAsync(ct).ConfigureAwait(false);
+            try
             {
-                EvictRuntime(device, parent.Name);
+                await runtime.ReadAsync(ct).ConfigureAwait(false);
-                var mapped = AbCipStatusMapper.MapLibplctagStatus(status);
-                StampGroupStatus(group, results, now, mapped);
-                _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead,
-                    $"libplctag status {status} reading UDT {group.ParentName}");
-                _logger.LogWarning(
-                    "AbCip whole-UDT read returned non-zero libplctag status {LibplctagStatus} for parent {Parent} " +
-                    "on device {Device}; {MemberCount} member values stamped with mapped status",
-                    status, group.ParentName, parent.DeviceHostAddress, group.Members.Count);
-                return;
-            }
 
-            foreach (var member in group.Members)
+                var status = runtime.GetStatus();
-            {
+                if (status != 0)
-                var value = runtime.DecodeValueAt(member.Definition.DataType, member.Offset, bitIndex: null);
+                {
-                results[member.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now);
+                    EvictRuntime(device, parent.Name);
+                    var mapped = AbCipStatusMapper.MapLibplctagStatus(status);
+                    StampGroupStatus(group, results, now, mapped);
+                    _health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead,
+                        $"libplctag status {status} reading UDT {group.ParentName}");
+                    _logger.LogWarning(
+                        "AbCip whole-UDT read returned non-zero libplctag status {LibplctagStatus} for parent {Parent} " +
+                        "on device {Device}; {MemberCount} member values stamped with mapped status",
+                        status, group.ParentName, parent.DeviceHostAddress, group.Members.Count);
+                    return;
+                }
+
+                foreach (var member in group.Members)
+                {
+                    var value = runtime.DecodeValueAt(member.Definition.DataType, member.Offset, bitIndex: null);
+                    results[member.OriginalIndex] = new DataValueSnapshot(value, AbCipStatusMapper.Good, now, now);
+                }
+                _health = new DriverHealth(DriverState.Healthy, now, null);
+            }
+            finally
+            {
+                opLock.Release();
             }
-            _health = new DriverHealth(DriverState.Healthy, now, null);
         }
         catch (OperationCanceledException)
         {
@@ -706,10 +730,23 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
                 }
 
                 var runtime = await EnsureTagRuntimeAsync(device, def, cancellationToken).ConfigureAwait(false);
-                runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value);
-                await runtime.WriteAsync(cancellationToken).ConfigureAwait(false);
 
-                var status = runtime.GetStatus();
+                // Serialise Encode → Write → GetStatus against the shared runtime — the same cached
+                // Tag handle may be in use by a concurrent ReadAsync or poll loop (05/STAB-4).
+                var opLock = device.GetRuntimeLock(def.Name);
+                await opLock.WaitAsync(cancellationToken).ConfigureAwait(false);
+                int status;
+                try
+                {
+                    runtime.EncodeValue(def.DataType, parsedPath?.BitIndex, w.Value);
+                    await runtime.WriteAsync(cancellationToken).ConfigureAwait(false);
+                    status = runtime.GetStatus();
+                }
+                finally
+                {
+                    opLock.Release();
+                }
+
                 if (status != 0)
                 {
                     EvictRuntime(device, def.Name);