Copilot commented on code in PR #3928:
URL: https://github.com/apache/avro/pull/3928#discussion_r3737457751
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -478,48 +483,57 @@ private FieldReader createArrayReader(Schema
readerSchema, Container action) thr
boolean zeroByteElements =
GenericDatumReader.isZeroByteSchema(elementType);
return reusingReader((reuse, decoder) -> {
- // Open a decode scope so the zero-byte element allocation cap is
cumulative
- // across every block of this array even when the fast reader is used
- // standalone (i.e. without GenericDatumReader.read opening the outer
datum
- // scope); otherwise a huge array split into many small blocks would
bypass
- // the cap. The scope nests: when a datum scope is already open this
simply
- // accumulates into it, and only the outermost scope resets the running
- // total (see SystemLimitException). The try/finally guarantees the
scope is
- // always closed so ThreadLocal state cannot leak into later decodes on
the
- // same thread.
- SystemLimitException.beginCollectionAllocationScope();
+ // Descending into an array grows the decode call stack; bound the
nesting
+ // depth first so a recursive schema cannot overflow the stack. Kept
outside
+ // the collection-allocation scope below so that when the depth check
throws
+ // (before incrementing) no unbalanced decrement occurs.
+ SystemLimitException.incrementDecodeDepth();
Review Comment:
In `createArrayReader(...)`, `incrementDecodeDepth()` is called before
`beginCollectionAllocationScope()`. When the fast reader is used standalone
(your comment says this is supported), `beginCollectionAllocationScope()` runs
at `scope.depth == 0` and resets `scope.decodeDepth` to 0, which (a) can wipe
out the just-incremented depth so the array descent is no longer counted, and
(b) can also cause false positives if a stale `decodeDepth` triggers the limit
*before* the scope reset would have cleared it. To make the reset effective and
keep the depth accounting consistent, open the collection-allocation scope
first (so it can reset stale depth at the datum boundary), then perform the
depth increment, and ensure both the depth decrement and scope end happen in
`finally` blocks even when the increment throws.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
