fix(WRK-22,WRK-24,WRK-25,WRK-27,IPC-26): worker write-seam hardening
WRK-22/IPC-26: tombstone a WriteAsync/WriteBatchAsync cancelled while waiting for the write lock (PendingFrame.Claimed under _gate; DequeueNext skips cancelled, claims the frame it returns) so a cancelled write never reaches the wire unless already claimed mid-write (documented residual). WRK-25: add WriteBatchAsync; RunEventDrainLoopAsync submits the drained event batch through it, so a burst of N events costs one flush not N. IPC-30 oversized-event structured fault preserved via FindOversizedEvent. WRK-24: reject a below-1024 negotiated frame maximum at the handshake (MinNegotiableFrameBytes, matching GatewayOptionsValidator floor). WRK-27: alarm poll advertises StaCallInProgress on the heartbeat snapshot so the watchdog suppresses to the ceiling, not the grace. Docs (WorkerFrameProtocol.md, MxAccessWorkerInstanceDesign.md) and the 2026-07-12 remediation registers/change-log updated in the same commit.
This commit is contained in:
@@ -18,6 +18,17 @@ public sealed class WorkerFrameProtocolOptions
|
||||
/// </summary>
|
||||
public const int MaxNegotiableFrameBytes = 256 * 1024 * 1024;
|
||||
|
||||
/// <summary>
|
||||
/// Lower floor the worker will accept for a gateway-negotiated frame maximum
|
||||
/// (<c>GatewayHello.max_frame_bytes</c>). Matches the gateway's own
|
||||
/// <c>GatewayOptionsValidator.MinimumMaxMessageBytes</c> validation floor so the worker never
|
||||
/// rejects a value the gateway's own validator accepts as legal configuration, yet a nonsensical
|
||||
/// tiny value (a gateway bug or a foreign/old peer) is rejected at the handshake rather than
|
||||
/// leaving a session that handshakes cleanly and then fails every subsequent frame with
|
||||
/// per-frame size errors. 1024 bytes still guarantees hellos, heartbeats, acks, and faults fit.
|
||||
/// </summary>
|
||||
public const int MinNegotiableFrameBytes = 1024;
|
||||
|
||||
/// <summary>Initializes a new instance of the WorkerFrameProtocolOptions class from WorkerOptions.</summary>
|
||||
/// <param name="options">Worker initialization options.</param>
|
||||
public WorkerFrameProtocolOptions(WorkerOptions options)
|
||||
@@ -118,7 +129,9 @@ public sealed class WorkerFrameProtocolOptions
|
||||
/// <summary>
|
||||
/// Adopts the gateway-negotiated frame maximum conveyed in <c>GatewayHello.max_frame_bytes</c>.
|
||||
/// A value of 0 (an older gateway that never set the field) is ignored and the
|
||||
/// constructor default is kept. A value above <see cref="MaxNegotiableFrameBytes"/> is rejected.
|
||||
/// constructor default is kept. A value outside the accepted range
|
||||
/// [<see cref="MinNegotiableFrameBytes"/>, <see cref="MaxNegotiableFrameBytes"/>] is rejected so a
|
||||
/// nonsensical negotiated value faults at the handshake rather than mid-session.
|
||||
/// </summary>
|
||||
/// <param name="negotiatedMaxFrameBytes">The gateway-negotiated maximum, or 0 for "keep default".</param>
|
||||
internal void AdoptNegotiatedMaxMessageBytes(uint negotiatedMaxFrameBytes)
|
||||
@@ -128,6 +141,14 @@ public sealed class WorkerFrameProtocolOptions
|
||||
return;
|
||||
}
|
||||
|
||||
if (negotiatedMaxFrameBytes < MinNegotiableFrameBytes)
|
||||
{
|
||||
throw new WorkerFrameProtocolException(
|
||||
WorkerFrameProtocolErrorCode.InvalidConfiguration,
|
||||
$"GatewayHello negotiated frame maximum {negotiatedMaxFrameBytes} is below the worker floor "
|
||||
+ $"of {MinNegotiableFrameBytes} bytes.");
|
||||
}
|
||||
|
||||
if (negotiatedMaxFrameBytes > MaxNegotiableFrameBytes)
|
||||
{
|
||||
throw new WorkerFrameProtocolException(
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.IO;
|
||||
using System.Runtime.ExceptionServices;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using Google.Protobuf;
|
||||
@@ -33,6 +34,16 @@ public sealed class WorkerFrameWriter
|
||||
|
||||
/// <summary>Gets the completion source signaled once the frame has been written or has failed.</summary>
|
||||
public TaskCompletionSource<bool> Completion { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Set to <c>true</c> by <see cref="DequeueNext"/> — under <c>_gate</c> — at the instant the
|
||||
/// draining lock-holder takes ownership of this frame to write it. A cancelled caller
|
||||
/// tombstones its frame only while it is still unclaimed, so a claim and a cancel can never
|
||||
/// both win: the flag is the interlock between the two. A frame already claimed is mid-write
|
||||
/// and can no longer be recalled (see <see cref="WriteAsync(WorkerEnvelope, WorkerFrameWritePriority, CancellationToken)"/>).
|
||||
/// Mutated only under <c>_gate</c>.
|
||||
/// </summary>
|
||||
public bool Claimed;
|
||||
}
|
||||
|
||||
private readonly WorkerFrameProtocolOptions _options;
|
||||
@@ -75,6 +86,13 @@ public sealed class WorkerFrameWriter
|
||||
/// <param name="priority">Scheduling priority; control frames are written ahead of event frames.</param>
|
||||
/// <param name="cancellationToken">Token to cancel waiting for the write lock.</param>
|
||||
/// <returns>A task that completes when the frame has been written and flushed.</returns>
|
||||
/// <remarks>
|
||||
/// Cancellation contract (WRK-22): if the token fires while this call is waiting for the write
|
||||
/// lock, the frame is tombstoned so it is never written — unless a draining lock-holder has
|
||||
/// already claimed it, in which case the frame may still reach the wire even though this call
|
||||
/// observes <see cref="OperationCanceledException"/>. That residual window is by design: blocking
|
||||
/// the canceller behind the very write it is abandoning would defeat the point of cancellation.
|
||||
/// </remarks>
|
||||
public async Task WriteAsync(
|
||||
WorkerEnvelope envelope,
|
||||
WorkerFrameWritePriority priority,
|
||||
@@ -101,7 +119,19 @@ public sealed class WorkerFrameWriter
|
||||
// Contend for the single writer: whoever wins drains every currently-queued frame in priority
|
||||
// order, so this frame is written by this call or by a concurrent caller that got the lock
|
||||
// first. Either way it completes via its own TaskCompletionSource.
|
||||
await _writeLock.WaitAsync(cancellationToken).ConfigureAwait(false);
|
||||
try
|
||||
{
|
||||
await _writeLock.WaitAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
// Tombstone the queued frame so DequeueNext skips it — but only if a draining lock-holder
|
||||
// has not already claimed it. If it is claimed it is mid-write and cannot be recalled; the
|
||||
// caller still observes cancellation while the frame reaches the wire (documented above).
|
||||
TombstoneIfUnclaimed(frame, cancellationToken);
|
||||
throw;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await DrainQueuedFramesAsync().ConfigureAwait(false);
|
||||
@@ -114,6 +144,123 @@ public sealed class WorkerFrameWriter
|
||||
await frame.Completion.Task.ConfigureAwait(false);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Queues a whole batch of envelopes at one priority under a single lock acquisition and drains
|
||||
/// it, so a burst of frames — the event drain loop's hot path — pays one flush for the batch
|
||||
/// rather than one per frame (WRK-25, realizing the WRK-12 coalescing on the path it was built
|
||||
/// for). Intra-batch order is preserved because the enqueue is atomic under <c>_gate</c> and each
|
||||
/// class queue is FIFO; the control-before-event guarantee still holds because any concurrently
|
||||
/// queued control frame is drained ahead of this batch by <see cref="DequeueNext"/>. Every frame's
|
||||
/// "written and flushed before completion" contract is unchanged.
|
||||
/// </summary>
|
||||
/// <param name="envelopes">Envelopes to write, in order.</param>
|
||||
/// <param name="priority">Scheduling priority for the whole batch.</param>
|
||||
/// <param name="cancellationToken">Token to cancel waiting for the write lock.</param>
|
||||
/// <returns>A task that completes when every frame in the batch has been written and flushed.</returns>
|
||||
/// <remarks>
|
||||
/// A per-frame rejection inside the batch (for example one oversized event) surfaces from the
|
||||
/// awaited completions as its <see cref="WorkerFrameProtocolException"/>; the remaining frames are
|
||||
/// still observed so none faults unobserved. Cancellation while waiting for the lock tombstones
|
||||
/// every still-unclaimed frame in the batch, per the WRK-22 contract on
|
||||
/// <see cref="WriteAsync(WorkerEnvelope, WorkerFrameWritePriority, CancellationToken)"/>.
|
||||
/// </remarks>
|
||||
public async Task WriteBatchAsync(
|
||||
IReadOnlyList<WorkerEnvelope> envelopes,
|
||||
WorkerFrameWritePriority priority,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
if (envelopes is null)
|
||||
{
|
||||
throw new ArgumentNullException(nameof(envelopes));
|
||||
}
|
||||
|
||||
if (envelopes.Count == 0)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
PendingFrame[] frames = new PendingFrame[envelopes.Count];
|
||||
for (int index = 0; index < envelopes.Count; index++)
|
||||
{
|
||||
WorkerEnvelope envelope = envelopes[index]
|
||||
?? throw new ArgumentException("Batch envelopes must not contain null.", nameof(envelopes));
|
||||
frames[index] = new PendingFrame(envelope);
|
||||
}
|
||||
|
||||
lock (_gate)
|
||||
{
|
||||
Queue<PendingFrame> queue = priority == WorkerFrameWritePriority.Event ? _eventFrames : _controlFrames;
|
||||
foreach (PendingFrame frame in frames)
|
||||
{
|
||||
queue.Enqueue(frame);
|
||||
}
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await _writeLock.WaitAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
TombstoneUnclaimed(frames, cancellationToken);
|
||||
throw;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await DrainQueuedFramesAsync().ConfigureAwait(false);
|
||||
}
|
||||
finally
|
||||
{
|
||||
_writeLock.Release();
|
||||
}
|
||||
|
||||
// Await every completion so no per-frame rejection faults unobserved, but surface the first
|
||||
// failure (in batch order) to the caller — the drain loop maps it back to the offending event.
|
||||
Exception? firstFailure = null;
|
||||
foreach (PendingFrame frame in frames)
|
||||
{
|
||||
try
|
||||
{
|
||||
await frame.Completion.Task.ConfigureAwait(false);
|
||||
}
|
||||
catch (Exception exception)
|
||||
{
|
||||
firstFailure ??= exception;
|
||||
}
|
||||
}
|
||||
|
||||
if (firstFailure is not null)
|
||||
{
|
||||
ExceptionDispatchInfo.Capture(firstFailure).Throw();
|
||||
}
|
||||
}
|
||||
|
||||
private void TombstoneIfUnclaimed(PendingFrame frame, CancellationToken cancellationToken)
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
if (!frame.Claimed)
|
||||
{
|
||||
frame.Completion.TrySetCanceled(cancellationToken);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private void TombstoneUnclaimed(PendingFrame[] frames, CancellationToken cancellationToken)
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
foreach (PendingFrame frame in frames)
|
||||
{
|
||||
if (!frame.Claimed)
|
||||
{
|
||||
frame.Completion.TrySetCanceled(cancellationToken);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Runs only under _writeLock. Drains control frames before event frames, stamping and writing each.
|
||||
// The stream write itself is not cancellable: a frame is written atomically or fails, never left
|
||||
// half-written on the pipe because a caller gave up waiting.
|
||||
@@ -198,18 +345,36 @@ public sealed class WorkerFrameWriter
|
||||
or WorkerFrameProtocolErrorCode.SessionMismatch;
|
||||
}
|
||||
|
||||
// Returns the next frame to write, control frames first, skipping any frame a cancelled caller
|
||||
// tombstoned while it waited for the lock (WRK-22). The frame actually returned is marked Claimed
|
||||
// under _gate in the same critical section that checks the tombstone, so a claim and a concurrent
|
||||
// cancel are mutually exclusive: whichever acquires _gate first wins.
|
||||
private PendingFrame? DequeueNext()
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
if (_controlFrames.Count > 0)
|
||||
while (_controlFrames.Count > 0)
|
||||
{
|
||||
return _controlFrames.Dequeue();
|
||||
PendingFrame frame = _controlFrames.Dequeue();
|
||||
if (frame.Completion.Task.IsCanceled)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
frame.Claimed = true;
|
||||
return frame;
|
||||
}
|
||||
|
||||
if (_eventFrames.Count > 0)
|
||||
while (_eventFrames.Count > 0)
|
||||
{
|
||||
return _eventFrames.Dequeue();
|
||||
PendingFrame frame = _eventFrames.Dequeue();
|
||||
if (frame.Completion.Task.IsCanceled)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
frame.Claimed = true;
|
||||
return frame;
|
||||
}
|
||||
|
||||
return null;
|
||||
|
||||
@@ -371,27 +371,57 @@ public sealed class WorkerPipeSession
|
||||
continue;
|
||||
}
|
||||
|
||||
foreach (WorkerEvent workerEvent in events)
|
||||
// Submit the whole drained batch through the writer's batch entry point under one lock
|
||||
// acquisition so the burst pays a single flush instead of one per event (WRK-25). Events
|
||||
// are the low-priority frame class: the writer holds them behind any pending control frame
|
||||
// (reply, fault, heartbeat, shutdown ack) so those are not delayed behind an event backlog,
|
||||
// and intra-batch order is preserved.
|
||||
WorkerEnvelope[] envelopes = new WorkerEnvelope[events.Count];
|
||||
for (int index = 0; index < events.Count; index++)
|
||||
{
|
||||
// Events are the low-priority frame class: the writer holds them behind any pending
|
||||
// control frame (reply, fault, heartbeat, shutdown ack) so those are not delayed
|
||||
// behind an event backlog.
|
||||
try
|
||||
{
|
||||
await _writer
|
||||
.WriteAsync(CreateEnvelope(workerEvent), WorkerFrameWritePriority.Event, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
catch (WorkerFrameProtocolException exception)
|
||||
when (exception.ErrorCode == WorkerFrameProtocolErrorCode.MessageTooLarge)
|
||||
{
|
||||
await FaultOnOversizedEventAsync(workerEvent, exception, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
envelopes[index] = CreateEnvelope(events[index]);
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await _writer
|
||||
.WriteBatchAsync(envelopes, WorkerFrameWritePriority.Event, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
catch (WorkerFrameProtocolException exception)
|
||||
when (exception.ErrorCode == WorkerFrameProtocolErrorCode.MessageTooLarge)
|
||||
{
|
||||
// A single oversized event surfaces from the batch's awaited completions; the death is
|
||||
// still IPC-30's structured, event-naming fault. Map the rejection back to the first
|
||||
// event in batch order whose envelope overshoots the negotiated maximum — the same
|
||||
// frame the writer rejected first.
|
||||
await FaultOnOversizedEventAsync(
|
||||
FindOversizedEvent(events, envelopes),
|
||||
exception,
|
||||
cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private WorkerEvent FindOversizedEvent(
|
||||
IReadOnlyList<WorkerEvent> events,
|
||||
WorkerEnvelope[] envelopes)
|
||||
{
|
||||
for (int index = 0; index < envelopes.Length; index++)
|
||||
{
|
||||
if (envelopes[index].CalculateSize() > _options.MaxMessageBytes)
|
||||
{
|
||||
return events[index];
|
||||
}
|
||||
}
|
||||
|
||||
// Unreachable in practice: WriteBatchAsync surfaced MessageTooLarge, so at least one envelope
|
||||
// exceeded the negotiated maximum. Fall back to the first event so the fault still names a
|
||||
// concrete event rather than throwing a second, less useful exception from the fault path.
|
||||
return events[0];
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Ends the session on an event that cannot be framed, but deliberately and diagnosably
|
||||
/// (IPC-30). An event above the negotiated frame maximum is undeliverable end to end — the
|
||||
@@ -1085,16 +1115,16 @@ public sealed class WorkerPipeSession
|
||||
return;
|
||||
}
|
||||
|
||||
if (!string.IsNullOrEmpty(snapshot.CurrentCommandCorrelationId)
|
||||
if ((!string.IsNullOrEmpty(snapshot.CurrentCommandCorrelationId) || snapshot.StaCallInProgress)
|
||||
&& staleFor <= _sessionOptions.HeartbeatStuckCeiling)
|
||||
{
|
||||
// A command is in flight and we are still within the defensive
|
||||
// suppression ceiling — the STA is busy executing it, not
|
||||
// hung. The next MarkActivity() in StaRuntime.ProcessQueuedCommands
|
||||
// will refresh LastActivityUtc once the command returns, at which
|
||||
// point this branch stops being taken. The heartbeat already
|
||||
// surfaces the in-flight correlation id so the gateway can apply
|
||||
// its own per-command timeout if it considers the command too slow.
|
||||
// A command is in flight, or an STA call outside the dispatcher (the alarm poll, WRK-27) is
|
||||
// executing, and we are still within the defensive suppression ceiling — the STA is busy
|
||||
// doing that work, not hung. The next MarkActivity() in StaRuntime.ProcessQueuedCommands
|
||||
// will refresh LastActivityUtc once the work returns, at which point this branch stops
|
||||
// being taken. The heartbeat already surfaces the in-flight correlation id so the gateway
|
||||
// can apply its own per-command timeout if it considers the command too slow; a poll that
|
||||
// blocks the STA past the ceiling still faults, which is the ceiling's contract.
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -24,6 +24,14 @@ public sealed class MxAccessStaSession : IWorkerRuntimeSession
|
||||
private CancellationTokenSource? alarmPollCts;
|
||||
private Task? alarmPollTask;
|
||||
private int? alarmConsumerThreadId;
|
||||
|
||||
// True on the STA thread exactly around the alarm PollOnce COM call. The alarm poll runs outside
|
||||
// the StaCommandDispatcher (so it does not inflate PendingCommandCount or perturb command dispatch
|
||||
// ordering), which means CaptureHeartbeat would otherwise see no in-flight activity during a long
|
||||
// poll and the watchdog would fault the session at the 15 s grace instead of the 75 s ceiling
|
||||
// granted to dispatched commands. Surfacing the poll on the heartbeat closes that asymmetry
|
||||
// (WRK-27). Volatile: written on the STA thread, read on the heartbeat thread.
|
||||
private volatile bool staAlarmPollInProgress;
|
||||
private bool disposed;
|
||||
|
||||
/// <summary>
|
||||
@@ -247,8 +255,20 @@ public sealed class MxAccessStaSession : IWorkerRuntimeSession
|
||||
await staRuntime.InvokeAsync(
|
||||
() =>
|
||||
{
|
||||
EnsureOnAlarmConsumerThread();
|
||||
handler.PollOnce();
|
||||
// Advertise the poll to the watchdog for exactly the span of the COM call
|
||||
// (WRK-27): set on the STA thread immediately before the affinity check and
|
||||
// PollOnce, clear in the finally so a heartbeat captured mid-poll reports
|
||||
// StaCallInProgress and one captured after does not.
|
||||
staAlarmPollInProgress = true;
|
||||
try
|
||||
{
|
||||
EnsureOnAlarmConsumerThread();
|
||||
handler.PollOnce();
|
||||
}
|
||||
finally
|
||||
{
|
||||
staAlarmPollInProgress = false;
|
||||
}
|
||||
},
|
||||
cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
@@ -377,7 +397,8 @@ public sealed class MxAccessStaSession : IWorkerRuntimeSession
|
||||
pendingCommandCount,
|
||||
(uint)eventQueue.Count,
|
||||
eventQueue.LastEventSequence,
|
||||
currentCommandCorrelationId);
|
||||
currentCommandCorrelationId,
|
||||
staAlarmPollInProgress);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -10,18 +10,26 @@ public sealed class WorkerRuntimeHeartbeatSnapshot
|
||||
/// <param name="outboundEventQueueDepth">Current depth of the worker event queue.</param>
|
||||
/// <param name="lastEventSequence">Sequence number of the most recent event.</param>
|
||||
/// <param name="currentCommandCorrelationId">Correlation ID of the in-flight command.</param>
|
||||
/// <param name="staCallInProgress">
|
||||
/// True while an STA call outside the command dispatcher is executing (currently the alarm poll,
|
||||
/// WRK-27). The watchdog treats this like an in-flight command: it suppresses the stale-STA fault
|
||||
/// up to the stuck ceiling instead of the shorter grace, so a healthy-but-slow poll does not fault
|
||||
/// a healthy session. Named generically so any future non-dispatcher STA work reuses it.
|
||||
/// </param>
|
||||
public WorkerRuntimeHeartbeatSnapshot(
|
||||
DateTimeOffset lastStaActivityUtc,
|
||||
uint pendingCommandCount,
|
||||
uint outboundEventQueueDepth,
|
||||
ulong lastEventSequence,
|
||||
string currentCommandCorrelationId)
|
||||
string currentCommandCorrelationId,
|
||||
bool staCallInProgress = false)
|
||||
{
|
||||
LastStaActivityUtc = lastStaActivityUtc;
|
||||
PendingCommandCount = pendingCommandCount;
|
||||
OutboundEventQueueDepth = outboundEventQueueDepth;
|
||||
LastEventSequence = lastEventSequence;
|
||||
CurrentCommandCorrelationId = currentCommandCorrelationId ?? string.Empty;
|
||||
StaCallInProgress = staCallInProgress;
|
||||
}
|
||||
|
||||
/// <summary>Gets the last STA activity timestamp in UTC.</summary>
|
||||
@@ -38,4 +46,11 @@ public sealed class WorkerRuntimeHeartbeatSnapshot
|
||||
|
||||
/// <summary>Gets the correlation ID of the in-flight command.</summary>
|
||||
public string CurrentCommandCorrelationId { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Gets a value indicating whether an STA call outside the command dispatcher (the alarm poll) is
|
||||
/// executing. When true the watchdog grants the poll the same grace-to-ceiling suppression as a
|
||||
/// dispatched command (WRK-27).
|
||||
/// </summary>
|
||||
public bool StaCallInProgress { get; }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user