Add MaxDepthBehavior for serializer recursion control
Introduced MaxDepthBehavior enum and option to control recursion depth handling (Truncate, Throw, Disable) in AcSerializerOptions. Refactored depth-check logic to use a precomputed NeedsDepthCheck flag. Enhanced exception messages for depth-limit violations. Updated tests to assert correct exception behavior for cycles. Improved documentation and added new test/log commands in settings.local.json.
This commit is contained in:
parent
ac6e66f59f
commit
6c61030c8a
File diff suppressed because one or more lines are too long
|
|
@ -544,13 +544,14 @@ public class AcBinarySourceGenerator : IIncrementalGenerator
|
||||||
sb.AppendLine();
|
sb.AppendLine();
|
||||||
sb.AppendLine(" public void WriteProperties<TOutput>(object value, AcBinarySerializer.BinarySerializationContext<TOutput> context) where TOutput : struct, IBinaryOutputBase");
|
sb.AppendLine(" public void WriteProperties<TOutput>(object value, AcBinarySerializer.BinarySerializationContext<TOutput> context) where TOutput : struct, IBinaryOutputBase");
|
||||||
sb.AppendLine(" {");
|
sb.AppendLine(" {");
|
||||||
sb.AppendLine(" // Global recursion depth safety net — only when ReferenceHandling != All");
|
sb.AppendLine(" // Global recursion depth safety net — gated by context.NeedsDepthCheck");
|
||||||
sb.AppendLine(" // (HasAllRefHandling=true tracks every type → write plan already prevents cycles via ObjectRef).");
|
sb.AppendLine(" // (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable).");
|
||||||
sb.AppendLine(" var needsDepthCheck = !context.HasAllRefHandling;");
|
sb.AppendLine(" // Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec.");
|
||||||
|
sb.AppendLine(" var needsDepthCheck = context.NeedsDepthCheck;");
|
||||||
sb.AppendLine(" if (needsDepthCheck)");
|
sb.AppendLine(" if (needsDepthCheck)");
|
||||||
sb.AppendLine(" {");
|
sb.AppendLine(" {");
|
||||||
sb.AppendLine(" if (context.RecursionDepth >= context.MaxDepth)");
|
sb.AppendLine(" if (context.RecursionDepth >= context.MaxDepth)");
|
||||||
sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary serialize: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at {ci.FullTypeName}\");");
|
sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary serialize: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at type '{ci.FullTypeName}' (depth=\" + context.RecursionDepth + \", position=\" + context.Position + \")\");");
|
||||||
sb.AppendLine(" context.RecursionDepth++;");
|
sb.AppendLine(" context.RecursionDepth++;");
|
||||||
sb.AppendLine(" }");
|
sb.AppendLine(" }");
|
||||||
sb.AppendLine();
|
sb.AppendLine();
|
||||||
|
|
@ -645,7 +646,7 @@ public class AcBinarySourceGenerator : IIncrementalGenerator
|
||||||
sb.AppendLine(" }");
|
sb.AppendLine(" }");
|
||||||
sb.AppendLine(" }");
|
sb.AppendLine(" }");
|
||||||
}
|
}
|
||||||
else if (ci.EnableRefHandling)
|
if (ci.EnableRefHandling && !ci.IsIId)
|
||||||
{
|
{
|
||||||
// Non-IId type: track via wrapper.TryTrackInt32 with RuntimeHelpers.GetHashCode
|
// Non-IId type: track via wrapper.TryTrackInt32 with RuntimeHelpers.GetHashCode
|
||||||
sb.AppendLine();
|
sb.AppendLine();
|
||||||
|
|
@ -676,16 +677,17 @@ public class AcBinarySourceGenerator : IIncrementalGenerator
|
||||||
(p.TypeKind == PropertyTypeKind.Collection && p.ElementKind == PropertyTypeKind.String && p.InterningFlags != 0) ||
|
(p.TypeKind == PropertyTypeKind.Collection && p.ElementKind == PropertyTypeKind.String && p.InterningFlags != 0) ||
|
||||||
(p.TypeKind == PropertyTypeKind.Dictionary && (p.DictKeyKind == PropertyTypeKind.String || p.DictValueKind == PropertyTypeKind.String) && p.InterningFlags != 0));
|
(p.TypeKind == PropertyTypeKind.Dictionary && (p.DictKeyKind == PropertyTypeKind.String || p.DictValueKind == PropertyTypeKind.String) && p.InterningFlags != 0));
|
||||||
|
|
||||||
// Global recursion depth safety net — only when ReferenceHandling != All
|
// Global recursion depth safety net — gated by context.NeedsDepthCheck
|
||||||
// (HasAllRefHandling=true tracks every type → 2nd-occurrence early-return already prevents cycles).
|
// (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable).
|
||||||
// Emitted AFTER all early returns (NeedsScan=false, feature-flag, null guard, IId 2nd-occurrence)
|
// Emitted AFTER all early returns (NeedsScan=false, feature-flag, null guard, IId 2nd-occurrence)
|
||||||
// and BEFORE the property scan loop that recurses into children.
|
// and BEFORE the property scan loop that recurses into children.
|
||||||
|
// Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec (JIT-independent guarantee).
|
||||||
sb.AppendLine();
|
sb.AppendLine();
|
||||||
sb.AppendLine(" var needsDepthCheck = !context.HasAllRefHandling;");
|
sb.AppendLine(" var needsDepthCheck = context.NeedsDepthCheck;");
|
||||||
sb.AppendLine(" if (needsDepthCheck)");
|
sb.AppendLine(" if (needsDepthCheck)");
|
||||||
sb.AppendLine(" {");
|
sb.AppendLine(" {");
|
||||||
sb.AppendLine(" if (context.RecursionDepth >= context.MaxDepth)");
|
sb.AppendLine(" if (context.RecursionDepth >= context.MaxDepth)");
|
||||||
sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary scan: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at {ci.FullTypeName}\");");
|
sb.AppendLine($" throw new System.InvalidOperationException(\"AcBinary scan: recursion depth exceeded MaxDepth=\" + context.MaxDepth + \" at type '{ci.FullTypeName}' (depth=\" + context.RecursionDepth + \")\");");
|
||||||
sb.AppendLine(" context.RecursionDepth++;");
|
sb.AppendLine(" context.RecursionDepth++;");
|
||||||
sb.AppendLine(" }");
|
sb.AppendLine(" }");
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -131,6 +131,19 @@ public class AcBinarySerializerIIdReferenceTests
|
||||||
|
|
||||||
Console.WriteLine($"\n========== ReferenceHandling: {options.ReferenceHandling}, UseSgen: {options.UseGeneratedCode}, UseMeta: {options.UseMetadata} ==========");
|
Console.WriteLine($"\n========== ReferenceHandling: {options.ReferenceHandling}, UseSgen: {options.UseGeneratedCode}, UseMeta: {options.UseMetadata} ==========");
|
||||||
|
|
||||||
|
// ReferenceHandling.None: cycle (Items[1].ParentOrder = order) is unprotected because no ref tracking.
|
||||||
|
// The global recursion depth safety net must throw before stack overflow (MaxDepth=10).
|
||||||
|
// Previous silent-null truncation (ACCORE-BIN-I-P2H8) caused data loss instead of surfacing the bug.
|
||||||
|
if (mode == ReferenceHandlingMode.None)
|
||||||
|
{
|
||||||
|
var ex = Assert.ThrowsExactly<InvalidOperationException>(() => AcBinarySerializer.Serialize(order, options), $"[{mode}] Expected InvalidOperationException for unprotected cycle, but Serialize succeeded");
|
||||||
|
|
||||||
|
StringAssert.Contains(ex.Message, "recursion depth exceeded", $"[{mode}] Exception message should mention recursion depth, got: {ex.Message}");
|
||||||
|
Console.WriteLine($"[{mode}] PASSED ✓ (depth safety net threw as expected: {ex.Message})");
|
||||||
|
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
var binary = AcBinarySerializer.Serialize(order, options);
|
var binary = AcBinarySerializer.Serialize(order, options);
|
||||||
//WriteBinaryToConsole(binary);
|
//WriteBinaryToConsole(binary);
|
||||||
|
|
@ -147,15 +160,6 @@ public class AcBinarySerializerIIdReferenceTests
|
||||||
// Assert based on mode
|
// Assert based on mode
|
||||||
switch (mode)
|
switch (mode)
|
||||||
{
|
{
|
||||||
case ReferenceHandlingMode.None:
|
|
||||||
//none esetén miért nincs infinite loop??? - J.
|
|
||||||
// Note: CountObjectRefs raw byte scan is unreliable in None mode —
|
|
||||||
// byte 65 (ObjectRef) == ASCII 'A', so "Product-A" and circular-ref
|
|
||||||
// depth expansion produce many false positives. Skip count assertion;
|
|
||||||
// data integrity checks below verify correct deserialization.
|
|
||||||
//WriteBinaryToConsole(binary);
|
|
||||||
break;
|
|
||||||
|
|
||||||
case ReferenceHandlingMode.OnlyId:
|
case ReferenceHandlingMode.OnlyId:
|
||||||
// sharedTag (Id=1) 4x → 3 ObjectRefs, sharedUser (Id=1) 2x → 1 ObjectRef = 4 total
|
// sharedTag (Id=1) 4x → 3 ObjectRefs, sharedUser (Id=1) 2x → 1 ObjectRef = 4 total
|
||||||
Assert.IsTrue(objectRefCount >= 4, $"[{mode}] Expected at least 4 ObjectRefs, found {objectRefCount}");
|
Assert.IsTrue(objectRefCount >= 4, $"[{mode}] Expected at least 4 ObjectRefs, found {objectRefCount}");
|
||||||
|
|
|
||||||
|
|
@ -40,7 +40,7 @@ public abstract class AcSerializerOptions
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Maximum depth for serialization/deserialization.
|
/// Maximum depth for serialization/deserialization.
|
||||||
/// 0 = root level only (primitives of root object)
|
/// 0 = depth check disabled (equivalent to <see cref="Serializers.MaxDepthBehavior.Disable"/>)
|
||||||
/// 1 = root + first level of nested objects/collections
|
/// 1 = root + first level of nested objects/collections
|
||||||
/// byte.MaxValue (255) = effectively unlimited
|
/// byte.MaxValue (255) = effectively unlimited
|
||||||
/// Default: byte.MaxValue
|
/// Default: byte.MaxValue
|
||||||
|
|
@ -51,6 +51,15 @@ public abstract class AcSerializerOptions
|
||||||
init => _maxDepth = value;
|
init => _maxDepth = value;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Action taken when serialization recursion depth reaches <see cref="MaxDepth"/>.
|
||||||
|
/// Default: <see cref="Serializers.MaxDepthBehavior.Throw"/> (cycle detection / bug surfacing).
|
||||||
|
/// Switch to <see cref="Serializers.MaxDepthBehavior.Truncate"/> for intentional shallow serialization
|
||||||
|
/// (client → server delta updates), or <see cref="Serializers.MaxDepthBehavior.Disable"/> for
|
||||||
|
/// max-perf when the graph is known cycle-free.
|
||||||
|
/// </summary>
|
||||||
|
public MaxDepthBehavior MaxDepthBehavior { get; init; } = MaxDepthBehavior.Throw;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Throw exception on circular reference detection for non-IId types.
|
/// Throw exception on circular reference detection for non-IId types.
|
||||||
/// When true: Tracks all objects and throws InvalidOperationException on circular references.
|
/// When true: Tracks all objects and throws InvalidOperationException on circular references.
|
||||||
|
|
@ -92,6 +101,41 @@ public enum AcSerializerType : byte
|
||||||
Toon = 2,
|
Toon = 2,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Behavior when serialization recursion depth reaches <see cref="AcSerializerOptions.MaxDepth"/>.
|
||||||
|
/// Controls whether the over-depth slice of the object graph is silently truncated, surfaces as
|
||||||
|
/// an exception, or whether the entire depth check is skipped for maximum throughput.
|
||||||
|
/// </summary>
|
||||||
|
public enum MaxDepthBehavior : byte
|
||||||
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// Replace the over-depth value with a <c>Null</c> marker in the wire stream — intentional shallow
|
||||||
|
/// serialization. Use when sending partial graphs (e.g., client → server delta updates, view-model
|
||||||
|
/// projections) where deep references are intentionally cut. The receiver reads <c>null</c> at the
|
||||||
|
/// truncation boundary: at the wire level this is indistinguishable from a genuine <c>null</c>, so
|
||||||
|
/// the consumer must encode that semantic in its protocol (e.g., "only modified properties are
|
||||||
|
/// persisted; nested nulls are skipped, not overwritten").
|
||||||
|
/// </summary>
|
||||||
|
Truncate = 0,
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Throw <see cref="System.InvalidOperationException"/> when depth limit is reached — cycle
|
||||||
|
/// detection and bug surfacing. Recommended default: an over-depth payload is almost always a bug
|
||||||
|
/// (circular reference without proper <see cref="ReferenceHandlingMode"/> tracking, or
|
||||||
|
/// pathologically deep graph). The exception message reports the offending type and depth so the
|
||||||
|
/// failure mode is debuggable rather than silent.
|
||||||
|
/// </summary>
|
||||||
|
Throw = 1,
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Skip the depth limit check entirely — maximum hot-path performance. Use only when the developer
|
||||||
|
/// guarantees the object graph is cycle-free and structurally bounded; otherwise the serializer
|
||||||
|
/// can stack-overflow on pathological input. Equivalent to setting <see cref="AcSerializerOptions.MaxDepth"/>=0
|
||||||
|
/// but expresses intent at the option level instead of via a magic depth value.
|
||||||
|
/// </summary>
|
||||||
|
Disable = 2,
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Reference handling mode for serialization.
|
/// Reference handling mode for serialization.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
|
|
||||||
|
|
@ -90,10 +90,31 @@ public static partial class AcBinarySerializer
|
||||||
/// Global recursion depth counter — final safety net against pathological graphs and non-IId cycles.
|
/// Global recursion depth counter — final safety net against pathological graphs and non-IId cycles.
|
||||||
/// Incremented/decremented at object-recursion entry/exit points (WriteObject runtime + generated WriteProperties/ScanObject).
|
/// Incremented/decremented at object-recursion entry/exit points (WriteObject runtime + generated WriteProperties/ScanObject).
|
||||||
/// Checked against <see cref="AcSerializerContextBase{TMetadata,TOptions}.MaxDepth"/> (byte, default 255).
|
/// Checked against <see cref="AcSerializerContextBase{TMetadata,TOptions}.MaxDepth"/> (byte, default 255).
|
||||||
/// Replaces the per-call <c>int depth</c> parameter passing — one byte field on context, ~5-7 cycles per object instead of per call.
|
/// Gated by <see cref="NeedsDepthCheck"/> — single context field-read at each call site.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
internal byte RecursionDepth;
|
internal byte RecursionDepth;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Pre-computed depth-check gate, set at <see cref="Reset"/>:
|
||||||
|
/// <c>!HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable</c>.
|
||||||
|
/// <list type="bullet">
|
||||||
|
/// <item><c>HasAllRefHandling=true</c>: every type tracked → 2nd-occurrence early-return prevents cycles → check redundant</item>
|
||||||
|
/// <item><c>MaxDepth=0</c>: explicit developer opt-out (magic value) — disables the safety net entirely</item>
|
||||||
|
/// <item><c>MaxDepthBehavior=Disable</c>: explicit developer opt-out (intent-level) — same effect, expressed via the behavior enum</item>
|
||||||
|
/// </list>
|
||||||
|
/// All three opt-outs collapse to <c>NeedsDepthCheck=false</c> so the hot path is a single field-read +
|
||||||
|
/// register-test, no per-call enum compare or negation. Truncate vs Throw dispatch (when implemented for
|
||||||
|
/// Truncate placement) consults <see cref="MaxDepthAction"/> on the rare depth-limit-hit path only.
|
||||||
|
/// </summary>
|
||||||
|
internal bool NeedsDepthCheck;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Pre-computed action when <see cref="RecursionDepth"/> reaches <see cref="AcSerializerContextBase{TMetadata,TOptions}.MaxDepth"/>.
|
||||||
|
/// Mirrors <see cref="AcSerializerOptions.MaxDepthBehavior"/>; checked inside the depth-comparison hot path
|
||||||
|
/// (gated short-circuit: comparison runs first, this enum is consulted only when the depth limit is actually hit).
|
||||||
|
/// </summary>
|
||||||
|
internal MaxDepthBehavior MaxDepthAction;
|
||||||
|
|
||||||
#region WriteDuplicateEntry — scan pass output for write pass cursor
|
#region WriteDuplicateEntry — scan pass output for write pass cursor
|
||||||
|
|
||||||
private WriteDuplicateEntry[]? _writePlan;
|
private WriteDuplicateEntry[]? _writePlan;
|
||||||
|
|
@ -299,6 +320,13 @@ public static partial class AcBinarySerializer
|
||||||
HasStringInterning = Options.UseStringInterning != StringInterningMode.None;
|
HasStringInterning = Options.UseStringInterning != StringInterningMode.None;
|
||||||
|
|
||||||
FastWire = Options.WireMode == WireMode.Fast;
|
FastWire = Options.WireMode == WireMode.Fast;
|
||||||
|
|
||||||
|
// Pre-compute depth-check gate. Three opt-out paths collapsed here:
|
||||||
|
// 1. ReferenceHandling=All → every type tracked, per-type 2nd-occurrence prevents cycles
|
||||||
|
// 2. MaxDepth=0 → explicit developer opt-out (magic value)
|
||||||
|
// 3. MaxDepthBehavior=Disable → explicit developer opt-out (intent-level enum)
|
||||||
|
MaxDepthAction = Options.MaxDepthBehavior;
|
||||||
|
NeedsDepthCheck = !HasAllRefHandling && Options.MaxDepth > 0 && MaxDepthAction != MaxDepthBehavior.Disable;
|
||||||
}
|
}
|
||||||
|
|
||||||
public override void Clear()
|
public override void Clear()
|
||||||
|
|
|
||||||
|
|
@ -156,12 +156,15 @@ public static partial class AcBinarySerializer
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fallback: runtime property loop (no SGen writer for this type)
|
// Fallback: runtime property loop (no SGen writer for this type)
|
||||||
// Global recursion depth safety net — only when ReferenceHandling != All
|
// Global recursion depth safety net — gated by context.NeedsDepthCheck
|
||||||
// (HasAllRefHandling=true tracks every type → 2nd-occurrence early-return above already prevents cycles).
|
// (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable).
|
||||||
var needsDepthCheck = !context.HasAllRefHandling;
|
// Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec.
|
||||||
|
var needsDepthCheck = context.NeedsDepthCheck;
|
||||||
if (needsDepthCheck)
|
if (needsDepthCheck)
|
||||||
{
|
{
|
||||||
if (context.RecursionDepth >= context.MaxDepth) throw new InvalidOperationException($"AcBinary scan: recursion depth exceeded MaxDepth={context.MaxDepth}");
|
if (context.RecursionDepth >= context.MaxDepth)
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
$"AcBinary scan: recursion depth exceeded MaxDepth={context.MaxDepth} at type '{wrapper.Metadata.SourceType.FullName}' (depth={context.RecursionDepth})");
|
||||||
context.RecursionDepth++;
|
context.RecursionDepth++;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1611,18 +1611,21 @@ public static partial class AcBinarySerializer
|
||||||
var generatedWriter = wrapper.GeneratedWriter;
|
var generatedWriter = wrapper.GeneratedWriter;
|
||||||
if (generatedWriter != null)
|
if (generatedWriter != null)
|
||||||
{
|
{
|
||||||
// SGen path handles its own RecursionDepth inc/dec via generated emit (gated on !HasAllRefHandling)
|
// SGen path handles its own RecursionDepth inc/dec via generated emit (gated by NeedsDepthCheck)
|
||||||
generatedWriter.WriteProperties(value, context);
|
generatedWriter.WriteProperties(value, context);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Runtime path: global recursion depth safety net — only when ReferenceHandling != All
|
// Runtime path: global recursion depth safety net — gated by context.NeedsDepthCheck
|
||||||
// (HasAllRefHandling=true tracks every type → write plan already prevents cycles via ObjectRef).
|
// (pre-computed at Reset(): !HasAllRefHandling && MaxDepth > 0 && MaxDepthBehavior != Disable).
|
||||||
var needsDepthCheck = !context.HasAllRefHandling;
|
// Local-cached flag: 1 ctx field-read, register-resident — re-used at inc and dec.
|
||||||
|
var needsDepthCheck = context.NeedsDepthCheck;
|
||||||
if (needsDepthCheck)
|
if (needsDepthCheck)
|
||||||
{
|
{
|
||||||
if (context.RecursionDepth >= context.MaxDepth) throw new InvalidOperationException($"AcBinary serialize: recursion depth exceeded MaxDepth={context.MaxDepth}");
|
if (context.RecursionDepth >= context.MaxDepth)
|
||||||
|
throw new InvalidOperationException($"AcBinary serialize: recursion depth exceeded MaxDepth={context.MaxDepth} at type '{wrapper.Metadata.SourceType.FullName}' (depth={context.RecursionDepth}, position={context.Position})");
|
||||||
|
|
||||||
context.RecursionDepth++;
|
context.RecursionDepth++;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue