diff --git a/code-reviews/Driver.AbCip/findings.md b/code-reviews/Driver.AbCip/findings.md index 21ec19c..387bd05 100644 --- a/code-reviews/Driver.AbCip/findings.md +++ b/code-reviews/Driver.AbCip/findings.md @@ -153,13 +153,13 @@ | Severity | Medium | | Category | Concurrency & thread safety | | Location | `AbCipDriver.cs:621-648`, `AbCipDriver.cs:591-614` | -| Status | Open | +| Status | Resolved | **Description:** `EnsureTagRuntimeAsync` and `EnsureParentRuntimeAsync` are check-then-act on a non-thread-safe `Dictionary` (`device.Runtimes` / `device.ParentRuntimes`). `ReadAsync` is `IReadable` and may be invoked concurrently: the server read path, each polled subscription loop, and the alarm projection poll loop all call `ReadAsync` independently. Two concurrent `ReadAsync` calls that both miss the cache for the same tag both create a `LibplctagTagRuntime`, both initialize it, and both write into the dictionary; the loser leaks an initialized native tag (never disposed, since only the dictionary value is disposed at shutdown), and concurrent `Dictionary` mutation can throw or corrupt the bucket structure. `WriteBitInDIntAsync` serializes the parent via a per-parent `SemaphoreSlim`, but `EnsureParentRuntimeAsync` still runs the same unguarded check-then-act on the shared `ParentRuntimes` dict. **Recommendation:** Use `ConcurrentDictionary` for `Runtimes` and `ParentRuntimes`, creating the runtime via `GetOrAdd` with a lazily-initialized factory, or guard the ensure path with a per-device lock / `SemaphoreSlim`. Ensure the losing creator runtime is disposed rather than leaked. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — already addressed as part of the Driver.AbCip-008 fix: `DeviceState.Runtimes` and `ParentRuntimes` were switched to `ConcurrentDictionary`; both `EnsureTagRuntimeAsync` and `EnsureParentRuntimeAsync` use the `TryGetValue` → create → `TryAdd` → dispose-loser pattern so concurrent callers that both miss the cache produce exactly one live runtime and the losing creator is disposed rather than leaked. ### Driver.AbCip-010