diff --git a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs index 95cbe13..1c79432 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Workers/WorkerFrameReader.cs @@ -5,11 +5,24 @@ using ZB.MOM.WW.MxGateway.Contracts.Proto; namespace ZB.MOM.WW.MxGateway.Server.Workers; +/// +/// Reads length-prefixed WorkerEnvelope protobuf frames from a stream. +/// +/// +/// is not reentrant: the reader keeps a per-instance length-prefix scratch +/// buffer, so exactly one consumer may be inside a read at a time. That matches how the reader is +/// used — a single read loop per WorkerClient, with handshake reads completing before the +/// loop starts. +/// public sealed class WorkerFrameReader { private readonly WorkerFrameProtocolOptions _options; private readonly Stream _stream; + // Reused across frames rather than allocated per read (GWC-30). Safe because ReadAsync is + // single-consumer by construction; the prefix is fully overwritten by every read. + private readonly byte[] _lengthPrefix = new byte[sizeof(uint)]; + /// /// Initializes a new instance of . /// @@ -30,10 +43,9 @@ public sealed class WorkerFrameReader /// Parsed worker envelope. public async ValueTask ReadAsync(CancellationToken cancellationToken = default) { - byte[] lengthPrefix = new byte[sizeof(uint)]; - await ReadExactlyOrThrowAsync(lengthPrefix, cancellationToken).ConfigureAwait(false); + await ReadExactlyOrThrowAsync(_lengthPrefix, cancellationToken).ConfigureAwait(false); - uint payloadLength = BinaryPrimitives.ReadUInt32LittleEndian(lengthPrefix); + uint payloadLength = BinaryPrimitives.ReadUInt32LittleEndian(_lengthPrefix); if (payloadLength == 0) { throw new WorkerFrameProtocolException( diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs index 910e77c..6663b0f 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/WorkerFrameProtocolTests.cs @@ -55,6 +55,41 @@ public sealed class WorkerFrameProtocolTests Assert.Equal(original, parsed); } + /// + /// One reader instance reads many frames in sequence. The reader reuses a single length-prefix + /// scratch buffer across calls (GWC-30), so varying the payload length frame to frame proves the + /// reused buffer is fully overwritten each time rather than carrying a stale prefix forward. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ReadAsync_WithMultipleFramesOnOneReader_ParsesEveryFrame() + { + const int frameCount = 5; + WorkerFrameProtocolOptions options = new(SessionId); + await using MemoryStream stream = new(); + WorkerFrameWriter writer = new(stream, options); + + List originals = []; + for (int index = 1; index <= frameCount; index++) + { + WorkerEnvelope envelope = CreateEnvelope(); + envelope.Sequence = (ulong)index; + + // Differing payload lengths so a stale length prefix would misparse rather than pass. + envelope.WorkerHello.WorkerVersion = new string('v', index * 37); + originals.Add(envelope); + await writer.WriteAsync(envelope); + } + + stream.Position = 0; + WorkerFrameReader reader = new(stream, options); + foreach (WorkerEnvelope original in originals) + { + WorkerEnvelope parsed = await reader.ReadAsync(); + Assert.Equal(original, parsed); + } + } + /// Verifies that reading a frame with partial reads reassembles the frame correctly. /// A task that represents the asynchronous operation. [Fact]