Fix RecyclerBytesStreamOutput allocating unlimited heap for some capacities #90632
Conversation
…cities We'd allocate infinite memory here because we'd overflow to negative capacities for newPosition close to `Integer.MAX_VALUE`.
original-brownbear
added
>bug
:Distributed/Network
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Hi @original-brownbear, I've created a changelog YAML for you. |
| // than Integer.MAX_VALUE | ||
| if (newPosition > Integer.MAX_VALUE - (Integer.MAX_VALUE % pageSize)) { | ||
| throw new IllegalArgumentException(getClass().getSimpleName() + " cannot hold more than 2GB of data"); | ||
| } |
There was a problem hiding this comment.
Should we convert org.elasticsearch.common.io.stream.RecyclerBytesStreamOutput#currentCapacity to long to prevent this overflow? It looks like it is not used anywhere else. Such change would also allow us to make above limit configurable when needed.
original-brownbear
added
the
auto-backport-and-merge
|
Thanks Ievgen, as discussed will address the suggestion in a follow-up. |
…cities (elastic#90632) We'd allocate infinite memory here because we'd overflow to negative capacities for newPosition close to `Integer.MAX_VALUE`.
💔 Backport failed
You can use sqren/backport to manually backport by running |
|
I think this deserves a test and it doesn't look too hard to do so without actually allocating GBs of heap - see #90638. |
|
@original-brownbear this is labelled 7.17 but I don't think |
We'd allocate infinite memory here because we'd overflow to negative capacities for newPosition close to
Integer.MAX_VALUE.