Copilot commented on code in PR #3927:
URL: https://github.com/apache/avro/pull/3927#discussion_r3731598301
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -481,33 +481,29 @@ private FieldReader createArrayReader(Schema
readerSchema, Container action) thr
if (reuse instanceof GenericArray) {
GenericArray<Object> reuseArray = (GenericArray<Object>) reuse;
long l = decoder.readArrayStart();
- long total = 0;
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
reuseArray.clear();
while (l > 0) {
for (long i = 0; i < l; i++) {
reuseArray.add(elementReader.read(reuseArray.peek(), decoder));
}
- total += l;
l = decoder.arrayNext();
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
}
return reuseArray;
} else {
long l = decoder.readArrayStart();
- long total = 0;
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
List<Object> array = (reuse instanceof List) ? (List<Object>) reuse
: new
GenericData.Array<>(GenericDatumReader.initialCollectionCapacity(l),
readerSchema);
array.clear();
while (l > 0) {
for (long i = 0; i < l; i++) {
array.add(elementReader.read(null, decoder));
}
- total += l;
l = decoder.arrayNext();
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
}
return array;
}
Review Comment:
If you add a try/finally scope around the fast array reader (see earlier
hunk), make sure the scope is always closed so ThreadLocal state can’t leak
into later decodes on the same thread. Closing it here also ensures the
zero-byte element accounting remains cumulative across all array blocks in this
read when the fast reader is used standalone.
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -481,33 +481,29 @@ private FieldReader createArrayReader(Schema
readerSchema, Container action) thr
if (reuse instanceof GenericArray) {
GenericArray<Object> reuseArray = (GenericArray<Object>) reuse;
long l = decoder.readArrayStart();
- long total = 0;
- checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
+ checkArrayBlock(decoder, elementType, zeroByteElements, l);
Review Comment:
FastReaderBuilder array decoding now relies on SystemLimitException’s
per-datum allocation scope to make the zero-byte allocation cap cumulative
across blocks. But FastReaderBuilder.createDatumReader(...) returns a
DatumReader that can be used directly (without GenericDatumReader.read’s
scope), so a large zero-byte array split across many blocks can bypass the cap
because checkMaxCollectionAllocation(count) becomes a per-block stateless check
when no scope is active. Consider opening a collection-allocation scope around
the array reader so the cap remains cumulative across array blocks even when
the fast reader is used standalone.
--
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]
