AyCode.Core/AyCode.Core/docs/BINARY/BINARY_ISSUES.md

22 KiB
Raw Blame History

Binary Serializer — Known Issues & Limitations

This page covers issues in the binary serializer core (format, SGen, options, deserialization context, buffer writer). Issues specific to the streaming I/O layer (AsyncPipeReaderInput + AsyncPipeWriterOutput, multi-message wire framing, sliding-window buffer, producer-consumer synchronization) are tracked separately in BINARY_ASYNCPIPE_ISSUES.md.

Deserialization

ACCORE-BIN-I-D2J5: Non-array-backed memory — per-segment copy

Status: Open Affects: SequenceBinaryInput Path: ExtractArray() fallback when MemoryMarshal.TryGetArray fails

When ReadOnlySequence<byte> segments are backed by native memory (not managed byte[]), each segment is copied into a new byte[]. This is unavoidable — the context requires byte[] for Unsafe.ReadUnaligned, AsSpan, and Encoding.GetString.

Impact: Negligible. Non-array-backed ReadOnlyMemory is extremely rare (custom MemoryManager<T> with native memory, memory-mapped files). All standard .NET pools (ArrayPool, MemoryPool.Shared, Kestrel pipe) are array-backed.

ACCORE-BIN-I-G7N3: Cross-boundary scratch buffer is not pooled across calls

Status: Open Affects: SequenceBinaryInput._scratchBuffer

The scratch buffer is ArrayPool.Rent-ed on first cross-boundary read and reused within a single deserialization. It is Return-ed in Release() after deserialization completes. However, the next deserialization will rent again.

Impact: Minimal. ArrayPool.Shared reuses buffers efficiently. The scratch is typically small (4-16 bytes for fixed-width boundary reads). Large scratch (>4KB) only occurs when a string or byte[] straddles a segment boundary.

Possible optimization: Store the scratch buffer on the pooled BinaryDeserializationContext and reuse across deserializations. Low priority — ArrayPool overhead is negligible.

ACCORE-BIN-I-S1F8: ReadBytes always copies

Status: Open Affects: BinaryDeserializationContext.ReadBytes(int length)

ReadBytes allocates a new byte[] and copies from the buffer. This is unavoidable because the caller owns the returned array, and the source buffer (pipe segment or serialized data) may be recycled.

ACCORE-BIN-I-V5L2: ReadStringUtf8 requires contiguous buffer

Status: Open Affects: BinaryDeserializationContext.ReadStringUtf8(int length)

Encoding.GetString and Ascii.IsValid require contiguous memory. For multi-segment reads, EnsureAvailable copies cross-boundary bytes into the scratch buffer first. This is the same approach SequenceReader<byte> uses internally.

Possible optimization: Span-by-span UTF-8 decode for cross-boundary strings (like MessagePack). Low priority — most strings are shorter than a segment (4KB).

Serialization

ACCORE-BIN-I-K8R4: BufferWriterBinaryOutput fallback path allocates per-chunk

Status: Open Affects: BufferWriterBinaryOutput.AcquireChunk fallback

When MemoryMarshal.TryGetArray fails on IBufferWriter.GetMemory() (native memory-backed writer), a byte[] is rented from ArrayPool per chunk and copied to the writer on Grow/Flush. Same as ACCORE-BIN-I-D2J5 — non-array-backed writers are extremely rare.

ACCORE-BIN-I-P3M6: AsyncPipeWriterOutput uses sync GetResult() for backpressure

Status: Open Affects: AsyncPipeWriterOutput.Grow()_lastFlush.GetAwaiter().GetResult()

When the previous PipeWriter.FlushAsync() hasn't completed by the next Grow() call, the serializer blocks the thread until the flush completes. This is necessary because IHubProtocol.WriteMessage is void (synchronous by design).

Impact: Minimal under normal conditions. PipeWriter.FlushAsync() writes to an in-memory Kestrel pipe (not directly to the network) and typically completes synchronously. Only blocks when the pipe's internal buffer hits its pause threshold (~1MB), which requires an extremely slow client + large payload. The Bytes mode (default) has the same blocking characteristic — it blocks the thread for the entire serialization + single flush.

Possible optimization: AsyncSegment mode (future) with a custom async WriteMessageAsync protocol interface, enabling await on flush instead of GetResult().

ACCORE-BIN-I-T9X1: AsyncPipeWriterOutput fallback path — same as ACCORE-BIN-I-K8R4

Status: Open Affects: AsyncPipeWriterOutput.AcquireChunk fallback

Same TryGetArray fallback as BufferWriterBinaryOutput (ACCORE-BIN-I-K8R4). Kestrel PipeWriter.GetMemory() always returns array-backed memory — fallback is for non-standard PipeWriter implementations only.

Deserialization (PipeReader)

ACCORE-BIN-I-B4Y7: PipeReaderBinaryInput uses sync ReadAsync().GetResult()

Status: Open Affects: PipeReaderBinaryInput.Initialize() and TryAdvanceSegment()

Same constraint as ACCORE-BIN-I-P3M6 — IBinaryInputBase interface is synchronous. ReadAsync().GetAwaiter().GetResult() blocks when waiting for more data from the pipe. Currently not used in production (SignalR delivers complete messages via TryParseMessage). Reserved for future direct-pipe deserialization scenarios.

Source Generator (SGen)

ACCORE-BIN-I-H2C5: CS8625 warnings for non-nullable reference types

Status: Open Affects: Generated reader code

The source generator emits null assignments for non-nullable reference type properties during deserialization (before the value is read from the stream). This produces CS8625 warnings. Functionally harmless — the property is always assigned before use.

ACCORE-BIN-I-N6Q3: First-run cold-start overhead

Status: Open Affects: First Serialize<T>/Deserialize<T> per [AcBinarySerializable] type, per process

Cold-start cost chain on first use of an SGen type (before ACCORE-BIN-T-W9F1 lands):

  1. BinarySerializeTypeMetadata ctor — reflection property enumeration + GetCustomAttribute scans
  2. Expression.Compile per property accessor (dynamic getter + typed getters) — dominant cost
  3. TypeMetadataWrapper ctor — GeneratedWriterRegistry + GeneratedReaderRegistry lookups, tracking state init
  4. JIT of WriteObject / WriteObjectProperties / scan pass
  5. JIT of generated WriteProperties / ScanObject / ScanForDuplicates (size scales with property count)
  6. Cascade: each referenced child type repeats steps 15

Subsequent calls hit cached metadata/wrappers → only Tier 0→1 JIT transition remains (background, async).

Dominant cost today: #1#2 (reflection + Expression.Compile). After ACCORE-BIN-T-W9F1, the dominant residual cost shifts to #4#5 (JIT), addressed by ACCORE-BIN-T-T5J8.

Impact: Measurable first-call latency — larger for types with many properties or deep graphs. For SignalR workloads the first message per entity type pays this tax.

ACCORE-BIN-I-F1W8: Consumer entity with new Id shadowing — excluded from SGen

Status: Open Affects: Any consumer entity whose base class hides BaseEntity.Id with readonly new int Id { get; } pattern (e.g. DiscountProductMapping in Mango.Nop.Core)

When the base class shadows Id with a setter-less new int Id { get; }, SGen can't emit a setter without CS0200. Runtime falls back to compiled-expression serialization for these types. Low priority — affects a small number of consumer entities.

Related TODO: BINARY_TODO.md#accore-bin-t-q2n7

Buffer Writer (BWO)

ACCORE-BIN-I-J4D2: Struct copy semantics

Status: Open Affects: BufferWriterBinaryOutput value-type assignment

Assigning a BufferWriterBinaryOutput value creates an independent copy. State changes (e.g. _committedBytes via Grow/Flush) are not reflected in the original. Copy back after use if needed.

ACCORE-BIN-I-R5V9: Initialize resets tracking

Status: Open Affects: BufferWriterBinaryOutput.Initialize (context mode)

Initialize sets _committedBytes = 0. Standalone bytes written before are lost if the BWO is then passed to a context. Call FlushAndReset() first, or track standalone bytes separately.

ACCORE-BIN-I-L7G3: Constructor acquires chunk

Status: Open Affects: BufferWriterBinaryOutput ctor

AcquireChunk runs in ctor for standalone readiness. Redundant if only context mode is used (context Initialize acquires its own). Not a leak — consecutive GetMemory without Advance returns overlapping memory.

ACCORE-BIN-I-M3K6: No mode mixing

Status: Open Affects: BufferWriterBinaryOutput — context vs standalone mode

A single instance must not use context + standalone modes simultaneously — buffer states desynchronize. One mode per lifecycle phase; FlushAndReset() as boundary between modes.

Configuration / Options

ACCORE-BIN-I-L8N5: AcBinarySerializerOptions thread-safety — mutable properties on shared instances

Status: Open Affects: AcBinarySerializerOptions — all set; properties (UseMetadata, UseGeneratedCode, WireMode, UseStringInterning, BufferWriterChunkSize, UseCompression) and any holder that retains a shared instance (e.g. DI-scoped serializer wrapper, long-lived service field, AcBinaryHubProtocol._options).

The options class exposes mutable properties. When a consumer shares one options instance across concurrent Serialize / Deserialize calls — common with DI-singleton services, hot-path-cached options, or any long-lived holder — a runtime property change is observable mid-operation by other in-flight calls. Result: invariant violations, mismatched encoding decisions, intermittent output / deserialization corruption.

Worsened by AcBinaryHubProtocol ctor (AyCode.Services/SignalRs/AcBinaryHubProtocol.cs sor 141), which mutates the caller-provided options reference (_options.BufferWriterChunkSize = options.BufferSize;) — the caller's external reference becomes a side-channel for the protocol's internal config.

A volatile field on the holder side (e.g. _options) only protects reference replacement, not property-level mutation; an external reference still in scope can be serOpts.UseGeneratedCode = false; mid-parse on another thread.

Impact: Latent — corruption is intermittent and timing-dependent; very hard to reproduce without targeted stress. The NuGet contract worsens this: the package cannot constrain how consumers scope their options instances.

Possible fix directions:

  • Defensive copy on ingressClone() on AcBinarySerializerOptions; every API that retains an options instance clones it on entry. External mutation becomes invisible to the holder.
  • Immutable record refactorset;init; on all configuration properties; mutation requires with-expression which produces a new instance.
  • Read-only flag pattern — à la JsonSerializerOptions.MakeReadOnly(). The holder calls MakeReadOnly() on entry; subsequent mutation throws.

ACCORE-BIN-I-C5R7: CheckDuplicatePropName=false silently corrupts on FNV-1a hash collision

Status: Open Affects: AcBinarySerializerOptions.CheckDuplicatePropName when set to false

The default value (true) throws InvalidOperationException on FNV-1a property-name hash collision within a type. When set to false (a doc-suggested production-performance optimization), collisions are silently accepted — the second property's hash overwrites the first in the lookup table, and the wrong property setter is invoked during deserialization. Result: silent data corruption between the colliding properties.

Impact: Latent — FNV-1a + typical property names rarely collide, but applications with many SGen types eventually hit one. Detection requires a separate property-by-property comparison after round-trip; the serializer surfaces no signal.

Possible fix directions:

  • Doc harmonization — single decision rule ("always true; perf cost is negligible vs. corruption risk").
  • Wider hash — replace FNV-1a with xxHash3-128 (or similar) for collision-free property identification.
  • Disambiguate by index — store property index alongside hash so a collision is detectable at deserialization without throwing on serialization.

ACCORE-BIN-I-P2H8: MaxDepth cut-off Null indistinguishable from real null

Status: Open Affects: AcBinarySerializerOptions.MaxDepth (and any preset using a non-default value, e.g. ShallowCopy preset has MaxDepth=0)

When the object graph exceeds MaxDepth, deeper objects/collections are written as Null(76)the same byte as a genuine null value. The deserializer cannot distinguish "depth-cut-off null" from "real null" → silent data loss without any signal at the receive side.

Impact: Latent for the default MaxDepth=255 (real graphs rarely hit). Severe with explicit lower limits — ShallowCopy preset (MaxDepth=0) silently drops every nested object on the receive side without an exception. Detection requires a separate depth-aware comparison.

Possible fix directions:

  • Dedicated DepthExceeded wire marker (distinct from Null(76)) — wire-format breaking change, major version.
  • Configurable policy on cut-offMaxDepthBehavior.WriteNull (today's default) / Throw / Log+WriteNull. Non-breaking opt-in.

ACCORE-BIN-I-W3F4: PropertyFilter + UseMetadata=false silently corrupts via index drift

Status: Open Affects: AcBinarySerializerOptions.PropertyFilter combined with UseMetadata=false

When the serializer applies a PropertyFilter, excluded properties are completely absent from the stream — no marker, no placeholder. The deserializer must apply an identical filter, OR rely on UseMetadata=true property-name hash matching. If neither condition holds, positional indices on the receive side mis-match: property A's value lands in property B's setter → silent data corruption.

Impact: Severe in NuGet contexts — the package cannot enforce symmetric filter configuration on both ends. A common pattern ("send-side filter to drop sensitive fields") silently corrupts cross-deployment if the receiver isn't aware to mirror.

Possible fix directions:

  • Emit PropertySkip(102) marker for filtered slots — the marker already exists on the wire, verify the write path uses it for filtered properties.
  • Auto-promote to UseMetadata=true when PropertyFilter is set (with a warning) — opt-out via explicit override.
  • Validate at serialize entryPropertyFilter != null && !UseMetadata → throw InvalidOperationException with guidance.

ACCORE-BIN-I-J6T9: Non-IId circular references silently truncated when ThrowOnCircularReference=false

Status: Open Affects: AcBinarySerializerOptions.ThrowOnCircularReference=false combined with ReferenceHandling != None

With ThrowOnCircularReference=false + reference handling enabled, only IId-implementing types are tracked for cycle detection. Non-IId circular references hit MaxDepth before being detected → silent truncation at the depth boundary, no exception, no log.

Impact: Borderline — explicit opt-in (developer must set ThrowOnCircularReference=false), and most domain models avoid non-IId cycles. But UI tree models (parent ↔ children with no Id), graph data structures, and self-referencing config nodes trigger this path silently.

Possible fix directions:

  • Universal cycle detection — track all reference types for cycle detection regardless of IId-ness when ThrowOnCircularReference=false (deduplication remains IId-only — cycle detection becomes universal).
  • Diagnostic event — surface "non-IId cycle dropped at depth N" as an Action<CycleDroppedDiagnostic>? on options, opt-in.

ACCORE-BIN-I-D9Y2: Default-value omission relies on type-level default consistency across writer/reader

Status: Open Affects: All property writes — BinaryTypeCode.PropertySkip (102) marker

The serializer writes a 1-byte PropertySkip marker for any property whose value equals default(T) instead of the full encoded value. The deserializer interprets this marker as "leave the target property at its CLR default" — which assumes the consumer-side type definition has the same default value as the producer-side.

Impact: Latent silent corruption across version-mismatched readers/writers. Three concrete failure modes:

  1. Default-value change in type definition: bool IsActive = true refactored to bool IsActive = false → wire produced with the old default decodes to wrong value on the new reader.
  2. Property added with non-zero default in v2: e.g., v2 adds int RetryCount = 3. Deserializing a v1 wire on the v2 reader → RetryCount = 0 (CLR default) instead of the expected v2 default 3.
  3. Cross-language consumers: a non-.NET reader following the wire spec must implement the same default-resolution rules per type — not portable, not self-describing on the wire.

Wire-size impact (production-realistic): Significant for DTOs with many optional/default-valued fields (status flags, nullable refs, default ints/decimals/Guids/DateTimes). Benchmark contribution: estimated ~3-8% of AcBinary wire-size advantage vs MemoryPack on the current test data; real-world DTOs may see 10-30% depending on the default-value share.

Possible fix directions (decision deferred — see BINARY_TODO.md#accore-bin-t-w7n5):

  • Doc-only: position as a deliberate protobuf-style feature, consumer responsibility to keep type definitions stable across versions.
  • Option flag: AcBinarySerializerOptions.OmitDefaults (default true for back-compat); false writes every property's full value regardless. Lets consumers opt out for fragile-class-evolution scenarios.
  • Hybrid: ship doc + flag, default true.

Wire Format / Cross-platform

ACCORE-BIN-I-E4N9: Wire format is host-native-endian, NOT canonical little-endian

Status: Open Affects: Serializer + Deserializer — all primitive readers/writers (Int16/UInt16/Int32/UInt32/Int64/UInt64/Float32/Float64/Decimal/Guid etc.), H2Q6 string tier headers (StringSmall/Medium/Big charLen+utf8Len pack), WireMode.Fast UTF-16 raw memcopy, every Unsafe.WriteUnaligned<T> / Unsafe.ReadUnaligned<T> call site.

The serializer/deserializer write/read multi-byte fields via Unsafe.WriteUnaligned<T>(ref byte, T) / Unsafe.ReadUnaligned<T>(ref byte), which use host-native endianness. On little-endian hosts (x86, x64, ARM64, WASM) this happens to match the wire-format-canonical little-endian; on big-endian hosts (PowerPC big-endian, MIPS big-endian, IBM-Z / S390x, SPARC big-endian) the bytes are reversed. A wire produced on a big-endian host cannot be read by a little-endian reader (and vice versa).

Impact: Cross-platform serialization between hosts of different endianness is currently silently broken — payloads decode with byte-reversed integers, floats, and headers. Same-endianness round-trips work correctly. Affects H2Q6 string tier headers (charLen / utf8Len byte-swap), all multi-byte primitives, and WireMode.Fast UTF-16 raw payloads.

Currently supported NuGet release platforms — all little-endian:

  • Cloud server (x64): Intel, AMD
  • Desktop (x64): Intel, AMD
  • Apple Silicon (ARM64): macOS, iOS, Mac Catalyst
  • Blazor WASM (WASM SIMD spec mandates little-endian)

Currently NOT supported (would silently emit/read wrong-endian wire on these hosts):

  • PowerPC big-endian (legacy IBM Power)
  • MIPS big-endian (legacy embedded)
  • IBM-Z / S390x (mainframe)
  • SPARC big-endian (legacy Sun/Oracle)

Possible fix directions:

  1. Document NuGet contract as little-endian-only — zero implementation cost, current code matches. NuGet readme + AcBinarySerializer XML doc-comment explicitly state the LE platform requirement; big-endian is "undefined behavior, wire silently incompatible". Pragmatic for the modern target platforms; matches the current de-facto reality.

  2. Defensive endian-guard at every Unsafe.*Unaligned call site:

    if (BitConverter.IsLittleEndian)
        Unsafe.WriteUnaligned<ushort>(ref _buffer[pos], value);
    else
        BinaryPrimitives.WriteUInt16LittleEndian(_buffer.AsSpan(pos, 2), value);
    

    BitConverter.IsLittleEndian is [Intrinsic] and constant-folds at JIT/AOT — zero cost on LE hosts (the else branch is dead-code-eliminated). On big-endian hosts the BinaryPrimitives.*LittleEndian family does the byte-swap via BinaryPrimitives.ReverseEndianness. Adds conditional code at all primitive r/w sites + H2Q6 headers (~10-15 sites).

  3. Replace all Unsafe.WriteUnaligned/ReadUnaligned with BinaryPrimitives.*LittleEndian unconditionally — these wrap Unsafe.WriteUnaligned + (conditional, JIT-folded on LE) ReverseEndianness. Slightly higher Span-creation overhead vs. direct ref byte writes; marginal perf cost on LE hosts. Over-engineering vs direction 2.

Pragmatic recommendation: Direction 1 (document) is the NuGet release minimum — explicitly state the LE constraint in the readme and XML doc-comments. Direction 2 (full endian-guard) is the comprehensive fix and should be a separate sprint covering ALL primitive r/w sites, H2Q6 string headers, WireMode.Fast UTF-16 path, and Float32/Float64/Decimal/Guid edge cases. A full BE-readiness audit is a non-trivial undertaking — only justified when there is concrete BE platform demand from a consumer.

Related TODO: worth tracking a follow-up TODO entry if direction 2 is chosen — covers writer + reader + SGen template + new round-trip tests on a BE-emulated host.

Cross-cutting (canonical home: ../XCUT/)

ACCORE-XCUT-I-X8Q1: JSON-in-Binary request parameters — cross-ref

Status: Closed (2026-04-26, see canonical entry).

Canonical entry: ../XCUT/XCUT_ISSUES.md#accore-xcut-i-x8q1. Summary: client→server request parameters previously used JSON inside a Binary envelope (SignalPostJsonDataMessage<T>); response path was already pure Binary. Migration landed in commits cdd54d3 + 3b70070 via SignalParams (length-prefixed binary pack/unpack) — wire is now Binary in both directions. Migration plan tracked in BINARY_TODO.md#accore-bin-t-s8p4 (also Closed).