dc2df628e3
The remediation reviews approved every task but left a tail of small notes. This lands the gateway-side half of them. Hardening (behavior changes, all narrow): - BuildFilteredWriteBulkCommand's unreachable default case failed OPEN: a fifth bulk-write kind added upstream without a filter case here would have shipped the DENIED entries to the worker while reporting them denied to the caller. It now throws UnreachableException. - SqliteCanonicalAuditStore.ListRecentAsync no longer throws on a row it cannot date. The retention sweep deliberately preserves such rows (SQLite's datetime() yields NULL, so the DELETE never matches), which guaranteed the dashboard's recent-audit view would meet one eventually and lose the whole page to it. The row is now reported at DateTimeOffset.MinValue with every other column intact, behind an optional logger. - The audit drain loop's finally now completes the channel writer alongside detaching the drain, so a producer that raced past the attached check takes the write-through branch instead of stranding its event in a buffer nobody reads until shutdown. TryComplete is idempotent, so StopAsync is unaffected. Tests: - MapCommandReply ownership (Assert.Same on the inner reply), mirroring the existing MapEvent ownership test. - Redactor key-id length boundary at exactly 64 and 65 characters, pinning which way it fails. Nothing validates key-id length at creation, so docs/Diagnostics.md's "which no issued key id does" is now stated as the heuristic it is. - ApiKeyFailureLimiter.Reset with a PartitionResolution whose partition was evicted between the Check and the Reset: inert, and clears nobody else's block. - Constraint-cache concurrency stress: the cap is enforced by the inserting thread, so overshoot must be transient and proportional to the in-flight inserters, and the cache must settle at or under the cap. - ListRecentAsync against a raw-SQL undateable row. Comment/doc accuracy: - EventsHubViewerRegistry.ReleaseConnection records that it relies on SignalR's default sequential per-connection dispatch (MaximumParallelInvocationsPerClient = 1). - A PERF(followup) note on Invoke's double session resolve and why removing it needs a SessionManager overload. - SessionEventDistributor: the volatile-field comment named the pump as the lock-free reader, but the pump's single capture point is inside _replayLock; the genuinely lock-free reader is SubscriberCount. OnSubscriberOverflow's "cannot be observed here" now excepts the DisposeAsync abandon path. The churn test names its ConcurrentDictionary bucket-order assumption and that a violation surfaces as a read timeout, not a silent pass. - The two "restores the sequential drain's behavior" claims (SessionManager, docs/Sessions.md) were wrong: the sequential drain leaked too, because KillWorkerAsync's entry ThrowIfCancellationRequested aborted the whole loop on the first session for zero kills. Reworded to "fixes a leak the sequential drain also had", with the sweep-bound/shutdown-unbound ParallelOptions asymmetry explained. - ISessionManager.ShutdownAsync's token doc: it degrades the drain to a kill sweep rather than cancelling it, with the bounded overrun stated. SessionShutdownHostedService.StopAsync records that its cancellation-logging branch is now unreachable.
153 lines
6.2 KiB
C#
153 lines
6.2 KiB
C#
using System.Collections.Concurrent;
|
|
|
|
namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
|
|
|
|
/// <summary>
|
|
/// Tracks which sessions currently have at least one live <see cref="EventsHub"/>
|
|
/// subscriber, so <see cref="DashboardEventBroadcaster"/> can skip the redaction
|
|
/// clone and the group send for sessions nobody is watching.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// SignalR does not expose group membership, so the hub mirrors its own
|
|
/// <c>AddToGroup</c>/<c>RemoveFromGroup</c> calls here. In the steady state no
|
|
/// browser is on a session-details page, yet every session's dashboard-mirror
|
|
/// subscriber still called <c>Publish</c> for every event — a deep protobuf
|
|
/// clone (values are redacted by default) plus a send to an empty group, per
|
|
/// event, thrown away. This registry is the cheap gate in front of that work.
|
|
/// <para>
|
|
/// Per-connection subscriptions are tracked as well, because a browser tab that
|
|
/// simply goes away never calls <c>UnsubscribeSession</c>; the hub's
|
|
/// <c>OnDisconnectedAsync</c> releases everything the connection held.
|
|
/// </para>
|
|
/// </remarks>
|
|
public sealed class EventsHubViewerRegistry
|
|
{
|
|
private readonly ConcurrentDictionary<string, int> _viewersBySession = new(StringComparer.Ordinal);
|
|
|
|
private readonly ConcurrentDictionary<string, ConcurrentDictionary<string, byte>> _sessionsByConnection =
|
|
new(StringComparer.Ordinal);
|
|
|
|
/// <summary>
|
|
/// Records that <paramref name="connectionId"/> is watching
|
|
/// <paramref name="sessionId"/>. Repeat calls for the same pair are
|
|
/// idempotent, so one <see cref="RemoveViewer"/> always clears them.
|
|
/// </summary>
|
|
/// <param name="connectionId">SignalR connection id of the subscriber.</param>
|
|
/// <param name="sessionId">Session id being watched.</param>
|
|
public void AddViewer(string connectionId, string sessionId)
|
|
{
|
|
if (string.IsNullOrWhiteSpace(connectionId) || string.IsNullOrWhiteSpace(sessionId))
|
|
{
|
|
return;
|
|
}
|
|
|
|
ConcurrentDictionary<string, byte> sessions = _sessionsByConnection.GetOrAdd(
|
|
connectionId,
|
|
static _ => new ConcurrentDictionary<string, byte>(StringComparer.Ordinal));
|
|
|
|
// The per-connection set is the source of truth for the count: only a
|
|
// subscription that was genuinely new increments the session's viewers.
|
|
if (!sessions.TryAdd(sessionId, 0))
|
|
{
|
|
return;
|
|
}
|
|
|
|
_viewersBySession.AddOrUpdate(sessionId, 1, static (_, count) => count + 1);
|
|
}
|
|
|
|
/// <summary>
|
|
/// Records that <paramref name="connectionId"/> stopped watching
|
|
/// <paramref name="sessionId"/>. A removal with no matching
|
|
/// <see cref="AddViewer"/> is a no-op, so the count cannot go negative.
|
|
/// </summary>
|
|
/// <param name="connectionId">SignalR connection id of the subscriber.</param>
|
|
/// <param name="sessionId">Session id no longer being watched.</param>
|
|
public void RemoveViewer(string connectionId, string sessionId)
|
|
{
|
|
if (string.IsNullOrWhiteSpace(connectionId) || string.IsNullOrWhiteSpace(sessionId))
|
|
{
|
|
return;
|
|
}
|
|
|
|
if (!_sessionsByConnection.TryGetValue(connectionId, out ConcurrentDictionary<string, byte>? sessions)
|
|
|| !sessions.TryRemove(sessionId, out _))
|
|
{
|
|
return;
|
|
}
|
|
|
|
ReleaseSession(sessionId);
|
|
}
|
|
|
|
/// <summary>
|
|
/// Releases every subscription held by <paramref name="connectionId"/>.
|
|
/// Called from the hub's disconnect callback, which is the only reliable
|
|
/// signal for a browser tab that closed without unsubscribing.
|
|
/// </summary>
|
|
/// <param name="connectionId">SignalR connection id that dropped.</param>
|
|
public void ReleaseConnection(string connectionId)
|
|
{
|
|
if (string.IsNullOrWhiteSpace(connectionId))
|
|
{
|
|
return;
|
|
}
|
|
|
|
// Detaching the set is safe against a SubscribeSession that arrives after the disconnect
|
|
// only because SignalR dispatches a connection's hub invocations sequentially by default
|
|
// (MaximumParallelInvocationsPerClient = 1): OnDisconnectedAsync cannot overlap an
|
|
// AddViewer for the same connection, so no late add can re-create the entry and leak a
|
|
// count that nothing will ever release. Raising that option would break this.
|
|
if (!_sessionsByConnection.TryRemove(connectionId, out ConcurrentDictionary<string, byte>? sessions))
|
|
{
|
|
return;
|
|
}
|
|
|
|
foreach (string sessionId in sessions.Keys)
|
|
{
|
|
// TryRemove, not a bare enumeration: a concurrent RemoveViewer on the
|
|
// same detached set must not let the session be decremented twice.
|
|
if (sessions.TryRemove(sessionId, out _))
|
|
{
|
|
ReleaseSession(sessionId);
|
|
}
|
|
}
|
|
}
|
|
|
|
/// <summary>Gets a value indicating whether any hub connection is watching the session.</summary>
|
|
/// <param name="sessionId">Session id to test.</param>
|
|
/// <returns><see langword="true"/> when at least one connection is subscribed.</returns>
|
|
public bool HasViewers(string sessionId) =>
|
|
!string.IsNullOrEmpty(sessionId)
|
|
&& _viewersBySession.TryGetValue(sessionId, out int count)
|
|
&& count > 0;
|
|
|
|
/// <summary>
|
|
/// Decrements the session's viewer count, dropping the entry entirely at
|
|
/// zero so the dictionary does not grow one key per session ever viewed.
|
|
/// The compare-and-swap loop keeps the decrement correct against a
|
|
/// concurrent <see cref="AddViewer"/> on the same session.
|
|
/// </summary>
|
|
/// <param name="sessionId">Session id whose count is released.</param>
|
|
private void ReleaseSession(string sessionId)
|
|
{
|
|
while (true)
|
|
{
|
|
if (!_viewersBySession.TryGetValue(sessionId, out int count))
|
|
{
|
|
return;
|
|
}
|
|
|
|
if (count <= 1)
|
|
{
|
|
if (_viewersBySession.TryRemove(new KeyValuePair<string, int>(sessionId, count)))
|
|
{
|
|
return;
|
|
}
|
|
}
|
|
else if (_viewersBySession.TryUpdate(sessionId, count - 1, count))
|
|
{
|
|
return;
|
|
}
|
|
}
|
|
}
|
|
}
|