diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowseOpenGuard.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowseOpenGuard.cs new file mode 100644 index 00000000..34c930f4 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Browsing/BrowseOpenGuard.cs @@ -0,0 +1,44 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; + +/// +/// Closes a browse session that finished opening after its host went away (R04 L4). Every picker and +/// RawBrowseModal awaited and then stored the token; +/// if the host was disposed (or the modal closed) meanwhile, its teardown had already run with no token, +/// so the session — an OPC UA client session, an MQTT connection, a SQL connection against the plant — +/// stayed open until the reaper's idle TTL (2 min + one 30 s tick). +/// +public static class BrowseOpenGuard +{ + /// + /// Call right after returns. When the host is gone it + /// closes the new session (fire-and-forget, like the hosts' own teardown) and returns + /// , telling the caller not to adopt the token. + /// + /// The open result. + /// Whether the host was disposed (or closed) while the open was in flight. + /// The browse service the session was opened on. + /// when the caller must drop the result. + public static bool CloseIfOrphaned(BrowseOpenResult result, bool hostGone, IBrowserSessionService service) + { + if (!hostGone) return false; + if (result.Ok && result.Token != Guid.Empty) + { + // Never block (or throw into) a dead host's continuation on a slow remote close. + _ = CloseQuietlyAsync(service, result.Token); + } + + return true; + } + + private static async Task CloseQuietlyAsync(IBrowserSessionService service, Guid token) + { + try + { + await service.CloseAsync(token).ConfigureAwait(false); + } + catch + { + // The reaper still bounds a session whose close failed. + } + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/AbCipAddressPickerBody.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/AbCipAddressPickerBody.razor index 2030e664..1f6cede6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/AbCipAddressPickerBody.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/AbCipAddressPickerBody.razor @@ -89,6 +89,8 @@ private string _built = ""; private Guid _token = Guid.Empty; + // Set first thing in DisposeAsync; an open that lands afterwards closes its own session. + private volatile bool _disposed; private bool _opening; private bool _canBrowse; private string? _openError; @@ -110,6 +112,8 @@ { var json = GetConfigJson() ?? "{}"; var result = await BrowserService.OpenAsync(DriverType, json, default); + // Host disposed mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed, BrowserService)) { return; } if (result.Ok) _token = result.Token; else _openError = result.Message; } @@ -155,6 +159,7 @@ public ValueTask DisposeAsync() { + _disposed = true; if (_token != Guid.Empty) { // Fire-and-forget — don't block circuit teardown on a slow capture. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor index 8edee5ad..bfc641f4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor @@ -164,6 +164,8 @@ private string _built = ""; private Guid _token = Guid.Empty; + // Set first thing in DisposeAsync; an open that lands afterwards closes its own session. + private volatile bool _disposed; private bool _opening; private bool _canBrowse; private string? _openError; @@ -185,6 +187,8 @@ { var json = GetConfigJson() ?? "{}"; var result = await BrowserService.OpenAsync(DriverType, json, default); + // Host disposed mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed, BrowserService)) { return; } if (result.Ok) _token = result.Token; else _openError = result.Message; } @@ -229,6 +233,7 @@ public ValueTask DisposeAsync() { + _disposed = true; if (_token != Guid.Empty) { // Fire-and-forget — don't block circuit teardown on a slow capture. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/GalaxyAddressPickerBody.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/GalaxyAddressPickerBody.razor index 0c3829d8..14fedd8c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/GalaxyAddressPickerBody.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/GalaxyAddressPickerBody.razor @@ -111,6 +111,8 @@ private string _attributeName = ""; private string _built = ""; private Guid _token = Guid.Empty; + // Set first thing in DisposeAsync; an open that lands afterwards closes its own session. + private volatile bool _disposed; private bool _opening; private bool _attrsLoading; private bool _canOperate; @@ -134,6 +136,8 @@ { var json = GetConfigJson() ?? "{}"; var result = await BrowserService.OpenAsync("GalaxyMxGateway", json, default); + // Host disposed mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed, BrowserService)) { return; } if (result.Ok) _token = result.Token; else _openError = result.Message; } @@ -216,6 +220,7 @@ public ValueTask DisposeAsync() { + _disposed = true; if (_token != Guid.Empty) { // Fire-and-forget — don't block circuit teardown on a slow remote. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/OpcUaClientAddressPickerBody.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/OpcUaClientAddressPickerBody.razor index 44daeeb2..02b79537 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/OpcUaClientAddressPickerBody.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/OpcUaClientAddressPickerBody.razor @@ -65,6 +65,8 @@ private string _nodeId = ""; private string _built = ""; private Guid _token = Guid.Empty; + // Set first thing in DisposeAsync; an open that lands afterwards closes its own session. + private volatile bool _disposed; private bool _opening; private bool _canOperate; private string? _openError; @@ -85,6 +87,8 @@ { var json = GetConfigJson() ?? "{}"; var result = await BrowserService.OpenAsync("OpcUaClient", json, default); + // Host disposed mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed, BrowserService)) { return; } if (result.Ok) _token = result.Token; else _openError = result.Message; } @@ -114,6 +118,7 @@ public ValueTask DisposeAsync() { + _disposed = true; if (_token != Guid.Empty) { // Fire-and-forget — don't block circuit teardown on a slow remote. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/SqlAddressPickerBody.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/SqlAddressPickerBody.razor index 11b925e0..61b445f1 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/SqlAddressPickerBody.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/SqlAddressPickerBody.razor @@ -240,6 +240,8 @@ else private string _built = ""; private string _selectedColumnNodeId = ""; private Guid _token = Guid.Empty; + // Set first thing in DisposeAsync; an open that lands afterwards closes its own session. + private volatile bool _disposed; private bool _opening; private bool _canOperate; private string? _openError; @@ -268,6 +270,8 @@ else // sole place it is read, and it dies with the local variable. var json = BuildBrowseConfig(); var result = await BrowserService.OpenAsync(DriverTypeNames.Sql, json, default); + // Host disposed mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed, BrowserService)) { return; } if (result.Ok) { _token = result.Token; } else { _openError = result.Message; } } @@ -429,6 +433,7 @@ else public ValueTask DisposeAsync() { + _disposed = true; if (_token != Guid.Empty) { // Fire-and-forget — don't block circuit teardown on a slow database. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/TwinCATAddressPickerBody.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/TwinCATAddressPickerBody.razor index 5306a27c..f5cab6a2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/TwinCATAddressPickerBody.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/TwinCATAddressPickerBody.razor @@ -76,6 +76,8 @@ private string _built = ""; private Guid _token = Guid.Empty; + // Set first thing in DisposeAsync; an open that lands afterwards closes its own session. + private volatile bool _disposed; private bool _opening; private bool _canBrowse; private string? _openError; @@ -97,6 +99,8 @@ { var json = GetConfigJson() ?? "{}"; var result = await BrowserService.OpenAsync(DriverType, json, default); + // Host disposed mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed, BrowserService)) { return; } if (result.Ok) _token = result.Token; else _openError = result.Message; } @@ -134,6 +138,7 @@ public ValueTask DisposeAsync() { + _disposed = true; if (_token != Guid.Empty) { // Fire-and-forget — don't block circuit teardown on a slow capture. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor index bb65b807..776a5b65 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor @@ -300,6 +300,8 @@ private string? _groupPrefix; private Guid _token = Guid.Empty; + // Set first thing in DisposeAsync; an open that lands afterwards closes its own session. + private volatile bool _disposed; private bool _opening; private string? _openError; @@ -459,6 +461,8 @@ try { var result = await BrowserService.OpenAsync(_driverType, _mergedConfig, default); + // Host disposed or modal closed mid-open: close the session nobody will (R04 L4). + if (BrowseOpenGuard.CloseIfOrphaned(result, _disposed || !Visible, BrowserService)) { return; } if (result.Ok) { _token = result.Token; @@ -823,6 +827,7 @@ public ValueTask DisposeAsync() { + _disposed = true; if (_token != Guid.Empty) { // Fire-and-forget — don't block circuit teardown on a slow remote. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs new file mode 100644 index 00000000..f744f815 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/BrowseOpenGuardTests.cs @@ -0,0 +1,169 @@ +using System.Reflection; +using System.Runtime.ExceptionServices; +using Microsoft.AspNetCore.Components; +using Microsoft.AspNetCore.Components.RenderTree; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.Browsing; +using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers.Pickers; +using ZB.MOM.WW.OtOpcUa.Commons.Browsing; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Pickers; + +// BL0006: the test renderer needs RenderTree types (same suppression as RawBrowseModalStaleOpenTests). +#pragma warning disable BL0006 + +/// +/// R04 L4: a browse session that finishes opening after its picker (or the browse modal) went away is +/// closed at once instead of living until the reaper's idle TTL. +/// +[Trait("Category", "Unit")] +public sealed class BrowseOpenGuardTests : IDisposable +{ + private readonly List _renderers = new(); + + public void Dispose() + { + foreach (var r in _renderers) r.Dispose(); + } + + [Fact] + public async Task Guard_closes_an_orphaned_session_and_leaves_a_live_one() + { + var svc = new RecordingBrowserService(); + var token = Guid.NewGuid(); + + BrowseOpenGuard.CloseIfOrphaned(new BrowseOpenResult(true, null, token), hostGone: false, svc).ShouldBeFalse(); + svc.Closed.ShouldBeEmpty(); + + BrowseOpenGuard.CloseIfOrphaned(new BrowseOpenResult(true, null, token), hostGone: true, svc).ShouldBeTrue(); + await svc.WaitForCloseAsync(); + svc.Closed.ShouldBe([token]); + } + + [Fact] + public void Guard_never_closes_a_failed_open_and_swallows_a_failing_close() + { + var svc = new RecordingBrowserService(); + BrowseOpenGuard.CloseIfOrphaned(new BrowseOpenResult(false, "refused", Guid.Empty), hostGone: true, svc).ShouldBeTrue(); + svc.Closed.ShouldBeEmpty(); + + var throwing = new RecordingBrowserService { ThrowOnClose = true }; + Should.NotThrow(() => BrowseOpenGuard.CloseIfOrphaned(new BrowseOpenResult(true, null, Guid.NewGuid()), true, throwing)); + } + + [Fact] + public async Task Picker_disposed_mid_open_closes_the_session_it_was_handed() + { + var gate = new TaskCompletionSource(); + var svc = new RecordingBrowserService { Open = () => gate.Task }; + var picker = NewPicker(svc); + + var open = InvokeOpen(picker); + await picker.DisposeAsync(); + var token = Guid.NewGuid(); + gate.SetResult(new BrowseOpenResult(true, null, token)); + await open; + + await svc.WaitForCloseAsync(); + svc.Closed.ShouldBe([token]); + Token(picker).ShouldBe(Guid.Empty); + } + + [Fact] + public async Task Picker_still_open_adopts_the_session() + { + // Positive control for the test above: without the dispose the token is adopted, not closed. + var token = Guid.NewGuid(); + var svc = new RecordingBrowserService { Open = () => Task.FromResult(new BrowseOpenResult(true, null, token)) }; + var picker = NewPicker(svc); + + await InvokeOpen(picker); + + Token(picker).ShouldBe(token); + svc.Closed.ShouldBeEmpty(); + } + + [Fact] + public void Every_browse_host_guards_its_open() + { + foreach (var path in new[] + { + "AbCipAddressPickerBody.razor", "FOCASAddressPickerBody.razor", "GalaxyAddressPickerBody.razor", + "OpcUaClientAddressPickerBody.razor", "TwinCATAddressPickerBody.razor", "SqlAddressPickerBody.razor", + }) + { + var src = AdminUiSource.Read("Components", "Shared", "Drivers", "Pickers", path); + src.ShouldContain("BrowseOpenGuard.CloseIfOrphaned(result, _disposed, BrowserService)", customMessage: path); + src.ShouldContain("_disposed = true;", customMessage: path); + } + + var modal = AdminUiSource.Read("Components", "Shared", "Raw", "RawBrowseModal.razor"); + modal.ShouldContain("BrowseOpenGuard.CloseIfOrphaned(result, _disposed || !Visible, BrowserService)"); + } + + private OpcUaClientAddressPickerBody NewPicker(IBrowserSessionService svc) + { + var picker = new OpcUaClientAddressPickerBody(); + typeof(OpcUaClientAddressPickerBody).GetProperty("BrowserService", BindingFlags.NonPublic | BindingFlags.Instance)! + .SetValue(picker, svc); + var renderer = new InertRenderer(); + _renderers.Add(renderer); + renderer.Attach(picker); + return picker; + } + + private static Task InvokeOpen(OpcUaClientAddressPickerBody picker) => + (Task)typeof(OpcUaClientAddressPickerBody).GetMethod("OpenBrowseAsync", BindingFlags.NonPublic | BindingFlags.Instance)! + .Invoke(picker, null)!; + + private static Guid Token(OpcUaClientAddressPickerBody picker) => + (Guid)typeof(OpcUaClientAddressPickerBody).GetField("_token", BindingFlags.NonPublic | BindingFlags.Instance)! + .GetValue(picker)!; + + private sealed class RecordingBrowserService : IBrowserSessionService + { + private readonly TaskCompletionSource _closed = new(TaskCreationOptions.RunContinuationsAsynchronously); + public List Closed { get; } = new(); + public bool ThrowOnClose { get; init; } + public Func> Open { get; init; } = () => Task.FromResult(new BrowseOpenResult(false, "n/a", Guid.Empty)); + + public Task WaitForCloseAsync() => _closed.Task.WaitAsync(TimeSpan.FromSeconds(10)); + + public bool CanBrowse(string driverType, string configJson) => true; + public Task OpenAsync(string driverType, string configJson, CancellationToken ct) => Open(); + public Task> RootAsync(Guid token, CancellationToken ct) => throw new NotImplementedException(); + public Task> ExpandAsync(Guid token, string nodeId, CancellationToken ct) => throw new NotImplementedException(); + public Task> AttributesAsync(Guid token, string nodeId, CancellationToken ct) => throw new NotImplementedException(); + public Task RequestRebirthAsync(Guid token, string scope, CancellationToken ct) => throw new NotImplementedException(); + public bool CanRequestRebirth(Guid token) => false; + + public Task CloseAsync(Guid token) + { + if (ThrowOnClose) throw new InvalidOperationException("remote gone"); + lock (Closed) Closed.Add(token); + _closed.TrySetResult(); + return Task.CompletedTask; + } + } + + private sealed class InertRenderer() + : Renderer(new ServiceCollection().BuildServiceProvider(), NullLoggerFactory.Instance) + { + public override Dispatcher Dispatcher { get; } = new InlineDispatcher(); + public void Attach(IComponent component) => AssignRootComponentId(component); + protected override void HandleException(Exception exception) => ExceptionDispatchInfo.Capture(exception).Throw(); + protected override Task UpdateDisplayAsync(in RenderBatch renderBatch) => Task.CompletedTask; + } + + private sealed class InlineDispatcher : Dispatcher + { + public override bool CheckAccess() => true; + public override Task InvokeAsync(Action workItem) { workItem(); return Task.CompletedTask; } + public override Task InvokeAsync(Func workItem) => workItem(); + public override Task InvokeAsync(Func workItem) => Task.FromResult(workItem()); + public override Task InvokeAsync(Func> workItem) => workItem(); + } +}