From 035bde0562a02dd1a0f9f121008724566fe489f9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 26 Jun 2026 17:55:44 -0400 Subject: [PATCH] fix(historian-gateway): dispose alarm-write channel at shutdown + ServerHistorian startup diagnostic Claude-Session: https://claude.ai/code/session_012SDSQ3AcaXqPcBtDESBRii --- .../SqliteStoreAndForwardSink.cs | 3 ++- .../GatewayAlarmHistorianWriter.cs | 9 ++++++++- src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs | 2 ++ .../GatewayAlarmHistorianWriterTests.cs | 16 ++++++++++++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs index c9bada22..f9a59c45 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.AlarmHistorian/SqliteStoreAndForwardSink.cs @@ -732,12 +732,13 @@ public sealed class SqliteStoreAndForwardSink : IAlarmHistorianSink, IDisposable /// Gets the current exponential backoff delay for retry operations. public TimeSpan CurrentBackoff => BackoffLadder[_backoffIndex]; - /// Disposes the sink and releases all held resources including the drain timer. + /// Disposes the sink and releases all held resources including the drain timer and the writer. public void Dispose() { if (_disposed) return; _disposed = true; _drainTimer?.Dispose(); _drainGate.Dispose(); + if (_writer is IDisposable writerDisposable) writerDisposable.Dispose(); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs index bddb63d5..02eb624a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/GatewayAlarmHistorianWriter.cs @@ -36,7 +36,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway; /// event forever. /// /// -public sealed class GatewayAlarmHistorianWriter : IAlarmHistorianWriter +public sealed class GatewayAlarmHistorianWriter : IAlarmHistorianWriter, IDisposable { private readonly IHistorianGatewayClient _client; private readonly ILogger _logger; @@ -154,4 +154,11 @@ public sealed class GatewayAlarmHistorianWriter : IAlarmHistorianWriter // Unknown/unclassified gRPC code → dead-letter to avoid an infinite drain loop. _ => HistorianWriteOutcome.PermanentFail, }; + + /// + /// Disposes the underlying gateway client and its gRPC channel. The concrete + /// implements ; test doubles + /// that only implement are safely no-opped by the cast guard. + /// + public void Dispose() => (_client as IDisposable)?.Dispose(); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index 14dd4575..09efb455 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -109,6 +109,8 @@ if (hasDriver) var serverHistorianOptions = builder.Configuration .GetSection(ServerHistorianOptions.SectionName).Get() ?? new ServerHistorianOptions(); + foreach (var warning in serverHistorianOptions.Validate()) + Log.Warning("ServerHistorian misconfiguration detected at startup: {Warning}", warning); builder.Services.AddAlarmHistorian( builder.Configuration, (_, sp) => GatewayHistorian.CreateAlarmWriter(serverHistorianOptions, sp)); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmHistorianWriterTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmHistorianWriterTests.cs index bd307d75..9477319e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmHistorianWriterTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/GatewayAlarmHistorianWriterTests.cs @@ -199,4 +199,20 @@ public sealed class GatewayAlarmHistorianWriterTests Assert.Empty(outcomes); } + + [Fact] + public void Dispose_with_async_only_client_does_not_throw() + { + // FakeHistorianGatewayClient implements IAsyncDisposable only — not IDisposable. + // The `as IDisposable` guard in GatewayAlarmHistorianWriter.Dispose() must safely + // no-op rather than throw when the client cannot be cast to IDisposable. + var fake = new FakeHistorianGatewayClient(); + var writer = Writer(fake); + + var ex = Record.Exception(() => ((IDisposable)writer).Dispose()); + + Assert.Null(ex); + // The Fake is IAsyncDisposable only; the sync Dispose must not call DisposeAsync. + Assert.Equal(0, fake.DisposeCallCount); + } }