From 60ffcfe8bd2ed76083c12c6b0bb34cefe23c8f3c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 09:26:08 -0400 Subject: [PATCH] fix(driver-ablegacy): resolve Medium code-review finding (Driver.AbLegacy-008) Mark _health volatile. The record-reference assignment is atomic, but without an acquire/release memory barrier GetHealth() on another thread can observe a stale snapshot indefinitely. volatile enforces the barrier at read and write sites without a lock. Co-Authored-By: Claude Opus 4.7 (1M context) --- code-reviews/Driver.AbLegacy/findings.md | 4 ++-- .../ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/code-reviews/Driver.AbLegacy/findings.md b/code-reviews/Driver.AbLegacy/findings.md index e855923..7e7b6ec 100644 --- a/code-reviews/Driver.AbLegacy/findings.md +++ b/code-reviews/Driver.AbLegacy/findings.md @@ -218,7 +218,7 @@ runtime of any race is disposed. | Severity | Medium | | Category | Concurrency & thread safety | | Location | `AbLegacyDriver.cs:21`, `AbLegacyDriver.cs:138-146`, `AbLegacyDriver.cs:216-229` | -| Status | Open | +| Status | Resolved | **Description:** `_health` is a plain non-volatile reference field mutated from `ReadAsync`, `WriteAsync` (both can run on multiple threads / poll loops) and @@ -233,7 +233,7 @@ successful read can clobber a `Degraded` write from a concurrent failing read. lock / `Interlocked.Exchange`. Consider only downgrading on failure and upgrading on a successful poll so a single failed read does not flap the surface. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — `_health` marked `volatile`; memory barrier comment documents the acquire/release ordering guarantee. ### Driver.AbLegacy-009 diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs index 3d94052..17d5759 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs @@ -17,7 +17,13 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover private readonly PollGroupEngine _poll; private readonly Dictionary _devices = new(StringComparer.OrdinalIgnoreCase); private readonly Dictionary _tagsByName = new(StringComparer.OrdinalIgnoreCase); - private DriverHealth _health = new(DriverState.Unknown, null, null); + + // volatile: _health is read by GetHealth() on any thread while ReadAsync / WriteAsync / + // InitializeAsync write it from worker / poll threads. The record-reference assignment is + // atomic on all .NET platforms, but without a memory barrier a reader can see a stale + // snapshot indefinitely. volatile enforces acquire/release ordering so GetHealth() always + // observes the most recently written value. + private volatile DriverHealth _health = new(DriverState.Unknown, null, null); public event EventHandler? OnDataChange; public event EventHandler? OnHostStatusChanged;