From 986dcee14a41627170da537fc0de284ee85f6ae8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 14 Jun 2026 02:35:59 -0400 Subject: [PATCH] worker(alarms): UnAdvise only advised handles in LmxSubtagAlarmSource teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B3: track advised handles separately from added handles so Dispose only UnAdvises items that were actually advised — a write-only subtag (e.g. ack-comment added by Write, never advised) is removed but not unadvised. Add Dispose tests covering the advised/write-only split, full removal, single Unregister, and double-dispose idempotency. --- .../MxAccess/LmxSubtagAlarmSourceTests.cs | 73 ++++++++++++++++--- .../MxAccess/LmxSubtagAlarmSource.cs | 16 +++- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/LmxSubtagAlarmSourceTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/LmxSubtagAlarmSourceTests.cs index bbfb05d..08995f9 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/LmxSubtagAlarmSourceTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/LmxSubtagAlarmSourceTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using ZB.MOM.WW.MxGateway.Worker.MxAccess; namespace ZB.MOM.WW.MxGateway.Worker.Tests.MxAccess; @@ -133,6 +134,60 @@ public sealed class LmxSubtagAlarmSourceTests Assert.Single(server.Writes); } + /// + /// Verifies UnAdvises only the + /// handles that were actually advised — a write-only item (added by + /// but never advised) is removed + /// but not unadvised — and unregisters the server exactly once. + /// + [Fact] + public void Dispose_UnAdvisesOnlyAdvisedHandles_RemovesAll_AndUnregistersOnce() + { + var server = new RecordingMxAccessServer(); + var source = new LmxSubtagAlarmSource(server, FakeServerHandle); + + source.Advise(new[] { "Tank1.Alarm.Subtag", "Tank2.Alarm.Subtag" }); + // A write-only subtag: added by Write, never advised. + source.Write("Tank1.Alarm.AckComment", "acknowledged"); + + int advised1 = server.LastItemHandleFor("Tank1.Alarm.Subtag"); + int advised2 = server.LastItemHandleFor("Tank2.Alarm.Subtag"); + int writeOnly = server.LastItemHandleFor("Tank1.Alarm.AckComment"); + + source.Dispose(); + + // Only the two advised handles are unadvised — never the write-only one. + Assert.Equal(new[] { advised1, advised2 }, server.UnAdvisedItemHandles); + Assert.DoesNotContain(writeOnly, server.UnAdvisedItemHandles); + // Every added item (advised + write-only) is removed. + Assert.Equal( + new[] { advised1, advised2, writeOnly }.OrderBy(h => h), + server.RemovedItemHandles.OrderBy(h => h)); + Assert.Equal(1, server.UnregisterCount); + } + + /// + /// Verifies is idempotent: a + /// second call performs no further teardown. + /// + [Fact] + public void Dispose_IsIdempotent() + { + var server = new RecordingMxAccessServer(); + var source = new LmxSubtagAlarmSource(server, FakeServerHandle); + + source.Advise(new[] { "Tank1.Alarm.Subtag" }); + + source.Dispose(); + int unadviseAfterFirst = server.UnAdvisedItemHandles.Count; + int unregisterAfterFirst = server.UnregisterCount; + + source.Dispose(); + + Assert.Equal(unadviseAfterFirst, server.UnAdvisedItemHandles.Count); + Assert.Equal(unregisterAfterFirst, server.UnregisterCount); + } + /// /// Recording test double that captures the /// AddItem/Advise/Write/UnAdvise/RemoveItem/Unregister calls @@ -152,13 +207,17 @@ public sealed class LmxSubtagAlarmSourceTests public List Writes { get; } = new(); + public List UnAdvisedItemHandles { get; } = new(); + + public List RemovedItemHandles { get; } = new(); + + public int UnregisterCount { get; private set; } + public int LastItemHandleFor(string itemAddress) => handlesByAddress[itemAddress]; public int Register(string clientName) => FakeServerHandle; - public void Unregister(int serverHandle) - { - } + public void Unregister(int serverHandle) => UnregisterCount++; public int AddItem(int serverHandle, string itemDefinition) { @@ -171,9 +230,7 @@ public sealed class LmxSubtagAlarmSourceTests public int AddItem2(int serverHandle, string itemDefinition, string itemContext) => AddItem(serverHandle, itemDefinition); - public void RemoveItem(int serverHandle, int itemHandle) - { - } + public void RemoveItem(int serverHandle, int itemHandle) => RemovedItemHandles.Add(itemHandle); public void Advise(int serverHandle, int itemHandle) { @@ -181,9 +238,7 @@ public sealed class LmxSubtagAlarmSourceTests AdvisedServerHandles.Add(serverHandle); } - public void UnAdvise(int serverHandle, int itemHandle) - { - } + public void UnAdvise(int serverHandle, int itemHandle) => UnAdvisedItemHandles.Add(itemHandle); public void AdviseSupervisory(int serverHandle, int itemHandle) { diff --git a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/LmxSubtagAlarmSource.cs b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/LmxSubtagAlarmSource.cs index c16e2e6..e334a9a 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/LmxSubtagAlarmSource.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker/MxAccess/LmxSubtagAlarmSource.cs @@ -46,6 +46,12 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource private readonly Dictionary addressesByItemHandle = new Dictionary(); + // Handles that were actually Advise()d, tracked separately from the added + // set so Dispose only UnAdvises advised items. Write() can AddItem a + // write-only subtag (e.g. an ack-comment that was never advised); calling + // UnAdvise on such a handle would be an unbalanced teardown. + private readonly HashSet advisedItemHandles = new HashSet(); + private object? mxAccessComObject; private IMxAccessServer? server; private LMXProxyServerClass? comEventSource; @@ -134,6 +140,7 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource mxServer.Advise(serverHandle, itemHandle); itemHandlesByAddress[itemAddress!] = itemHandle; addressesByItemHandle[itemHandle] = itemAddress!; + advisedItemHandles.Add(itemHandle); } } @@ -225,7 +232,13 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource { foreach (KeyValuePair entry in addressesByItemHandle) { - try { mxServer.UnAdvise(serverHandle, entry.Key); } catch { /* swallow — best effort */ } + // Only UnAdvise handles that were actually advised; a write-only + // item (added by Write but never Advise'd) was never advised. + if (advisedItemHandles.Contains(entry.Key)) + { + try { mxServer.UnAdvise(serverHandle, entry.Key); } catch { /* swallow — best effort */ } + } + try { mxServer.RemoveItem(serverHandle, entry.Key); } catch { /* swallow — best effort */ } } @@ -234,6 +247,7 @@ public sealed class LmxSubtagAlarmSource : ISubtagAlarmSource itemHandlesByAddress.Clear(); addressesByItemHandle.Clear(); + advisedItemHandles.Clear(); object? comToRelease = mxAccessComObject; mxAccessComObject = null;