Files
ScadaBridge/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/DebugStreamHubSubscribeRaceTests.cs
T
Joseph Doherty 561582747e fix(centralui): stop debug-stream sessions abandoned mid-start — dispose-vs-connect race (arch-review 2026-08-19 P6-6)
A debug-stream session created after the consumer had already gone away was
leaked permanently. Both consumers awaited StartStreamAsync (up to 30s) and only
then published the session; a circuit teardown / client disconnect inside that
window ran the release path first, found nothing registered and stopped nothing.
The session stayed registered in the singleton DebugStreamService, whose
keepalive renews registered sessions forever (the 5-minute orphan reaper only
collects UNregistered ones), pinning the site gRPC subscription, the
DebugStreamBridgeActor and the dead consumer's object graph until process
restart.

Both call sites now publish the session and re-check liveness under a single
monitor, so exactly one side tears it down and neither can miss it:

- DebugView.Connect/Dispose/Disconnect share a _sessionLock; Dispose sets
  _disposed and takes the session in one step, Connect publishes only if not
  disposed and otherwise stops the session it just created. Dispose can run off
  the renderer's thread, so the Blazor sync context is not relied on. Disconnect
  and the onTerminated callback go through the same take-once helper, which also
  makes a repeated Dispose stop-once instead of stop-again.
- DebugStreamHub uses the per-connection Items monitor: OnDisconnectedAsync marks
  the connection gone before releasing, SubscribeInstance publishes only if
  neither that flag nor ConnectionAborted is set, and UnsubscribeInstance takes
  the id atomically.

DebugStreamService.StartStreamAsync/StopStream become virtual purely as a test
seam (no behaviour change, keepalive/orphan contract untouched) so the race can
be driven deterministically by a gated stub.

Tests: DebugViewConnectDisposeRaceTests (3) + DebugStreamHubSubscribeRaceTests
(3). Verified fail-unfixed: with the two source fixes stashed, both
dispose-mid-start tests report zero StopStream calls and the DebugView
double-dispose test reports two.
2026-08-19 14:53:01 -04:00

180 lines
7.5 KiB
C#

using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Connections.Features;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.SignalR;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using NSubstitute;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.DebugView;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.Streaming;
using ZB.MOM.WW.ScadaBridge.Communication;
using ZB.MOM.WW.ScadaBridge.Communication.Grpc;
namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests;
/// <summary>
/// Regression tests for arch-review 2026-08-19 P6-6 on the hub side. <c>SubscribeInstance</c>
/// awaits <c>StartStreamAsync</c> — up to 30s — and only then stores the session id on the
/// connection. A client that disconnects inside that window ran <c>OnDisconnectedAsync</c>
/// while there was still no session id to release, so the session stayed registered in the
/// singleton <see cref="DebugStreamService"/> and its keepalive renewed it forever (the
/// orphan reaper only collects UNregistered sessions). The fix publishes the id and
/// re-checks connection liveness under the per-connection <c>Items</c> monitor, so exactly
/// one side tears the session down.
/// </summary>
public class DebugStreamHubSubscribeRaceTests
{
private sealed class GatedDebugStreamService : DebugStreamService
{
private readonly TaskCompletionSource _gate = new(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly TaskCompletionSource _entered = new(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly List<string> _stopped = new();
public GatedDebugStreamService()
: base(
new CommunicationService(Options.Create(new CommunicationOptions()), NullLogger<CommunicationService>.Instance),
new ServiceCollection().BuildServiceProvider(),
new SiteStreamGrpcClientFactory(NullLoggerFactory.Instance),
NullLogger<DebugStreamService>.Instance)
{
}
public string SessionId => "session-under-test";
/// <summary>Completes once <see cref="StartStreamAsync"/> has been entered.</summary>
public Task Entered => _entered.Task;
/// <summary>Session ids passed to <see cref="StopStream"/>, in call order.</summary>
public IReadOnlyList<string> Stopped
{
get { lock (_stopped) { return _stopped.ToList(); } }
}
public void ReleaseStart() => _gate.TrySetResult();
public override async Task<DebugStreamSession> StartStreamAsync(
int instanceId, Action<object> onEvent, Action onTerminated, CancellationToken ct = default)
{
_entered.TrySetResult();
await _gate.Task;
return new DebugStreamSession(
SessionId,
new DebugViewSnapshot(
"Inst-1",
Array.Empty<AttributeValueChanged>(),
Array.Empty<AlarmStateChanged>(),
DateTimeOffset.UtcNow));
}
public override void StopStream(string sessionId)
{
lock (_stopped) { _stopped.Add(sessionId); }
}
}
private sealed class TestHubCallerContext : HubCallerContext
{
private readonly FeatureCollection _features = new();
private readonly CancellationTokenSource _aborted = new();
public TestHubCallerContext(HttpContext http) =>
_features.Set<IHttpContextFeature>(new TestHttpContextFeature { HttpContext = http });
public override string ConnectionId => "test-conn";
public override string? UserIdentifier => null;
public override System.Security.Claims.ClaimsPrincipal? User => null;
public override IDictionary<object, object?> Items { get; } = new Dictionary<object, object?>();
public override IFeatureCollection Features => _features;
public override CancellationToken ConnectionAborted => _aborted.Token;
public override void Abort() => _aborted.Cancel();
private sealed class TestHttpContextFeature : IHttpContextFeature { public HttpContext? HttpContext { get; set; } }
}
private const string SessionIdKey = "DebugStreamSessionId";
private static (DebugStreamHub Hub, TestHubCallerContext Context, GatedDebugStreamService Stream) CreateHub()
{
var repo = Substitute.For<ITemplateEngineRepository>();
repo.GetInstanceByIdAsync(7).Returns(new Instance("Inst-1") { Id = 7, SiteId = 1 });
var services = new ServiceCollection();
services.AddSingleton(repo);
var http = new DefaultHttpContext { RequestServices = services.BuildServiceProvider() };
var stream = new GatedDebugStreamService();
var hub = new DebugStreamHub(
stream, Substitute.For<IHubContext<DebugStreamHub>>(), NullLogger<DebugStreamHub>.Instance)
{
Clients = Substitute.For<IHubCallerClients>()
};
var context = new TestHubCallerContext(http);
hub.Context = context;
// No roles / no permitted sites on the connection == system-wide scope, so the
// authorization branch is not what these tests are exercising.
return (hub, context, stream);
}
[Fact]
public async Task SubscribeInstance_ClientDisconnectsMidStart_StopsTheJustCreatedSession()
{
var (hub, context, stream) = CreateHub();
var subscribe = hub.SubscribeInstance(7);
await stream.Entered.WaitAsync(TimeSpan.FromSeconds(5));
// The client goes away while the start is still in flight — the disconnect path
// finds nothing registered yet.
await hub.OnDisconnectedAsync(null);
Assert.Empty(stream.Stopped);
stream.ReleaseStart();
await subscribe.WaitAsync(TimeSpan.FromSeconds(10));
Assert.Equal(new[] { stream.SessionId }, stream.Stopped);
Assert.False(context.Items.ContainsKey(SessionIdKey));
}
[Fact]
public async Task SubscribeInstance_ConnectionAbortedMidStart_StopsTheJustCreatedSession()
{
// Same window, but the abort token trips without OnDisconnectedAsync having run yet
// (SignalR aborts the connection before dispatching the disconnect handler).
var (hub, context, stream) = CreateHub();
var subscribe = hub.SubscribeInstance(7);
await stream.Entered.WaitAsync(TimeSpan.FromSeconds(5));
context.Abort();
stream.ReleaseStart();
await subscribe.WaitAsync(TimeSpan.FromSeconds(10));
Assert.Equal(new[] { stream.SessionId }, stream.Stopped);
Assert.False(context.Items.ContainsKey(SessionIdKey));
}
[Fact]
public async Task SubscribeInstance_ThenDisconnect_StopsTheSessionExactlyOnce()
{
var (hub, context, stream) = CreateHub();
stream.ReleaseStart();
await hub.SubscribeInstance(7);
Assert.Empty(stream.Stopped);
Assert.Equal(stream.SessionId, context.Items[SessionIdKey]);
await hub.OnDisconnectedAsync(null);
Assert.Equal(new[] { stream.SessionId }, stream.Stopped);
Assert.False(context.Items.ContainsKey(SessionIdKey));
// A second release (an explicit unsubscribe racing the disconnect) is a no-op.
await hub.UnsubscribeInstance();
Assert.Equal(new[] { stream.SessionId }, stream.Stopped);
}
}