Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Java] Accounted size went negative when locallyHeldMemory overflow #35960

Closed
dafeiw opened this issue Jun 7, 2023 · 1 comment · Fixed by #36185
Closed

[Java] Accounted size went negative when locallyHeldMemory overflow #35960

dafeiw opened this issue Jun 7, 2023 · 1 comment · Fixed by #36185

Comments

@dafeiw
Copy link

dafeiw commented Jun 7, 2023

Describe the bug, including details regarding any error messages, version, and platform.

Bug description:

The following code will throw Accounted size went negative exception.
The buffer allocator a will ask its parent allocator to do allocation, however the locallyHeldMemory of parent allocator already reaches Long.MAX_VALUE when creating child actor ChildB

        RootAllocator parentAllocator  = new RootAllocator();
        BufferAllocator a = parentAllocator.newChildAllocator("ChildA", 0, Long.MAX_VALUE);
        BufferAllocator b = parentAllocator.newChildAllocator("ChildB", Long.MAX_VALUE, Long.MAX_VALUE);
        IntVector intVector = new IntVector("intVector", a);
        intVector.allocateNew(1);
        intVector.set(0, 2);
        intVector.setValueCount(1);

        IntVector intVector2 = new IntVector("intVector2", a);
        intVector2.allocateNew(1);
        intVector2.set(0, 2);
        intVector2.setValueCount(1);

        intVector.close();
        intVector2.close();
        a.close();
        b.close();

error message:

Exception in thread "main" java.lang.IllegalArgumentException: Accounted size went negative.
	at org.apache.arrow.util.Preconditions.checkArgument(Preconditions.java:136)
	at org.apache.arrow.memory.Accountant.releaseBytes(Accountant.java:214)
	at org.apache.arrow.memory.RootAllocator.releaseBytes(RootAllocator.java:29)
	at org.apache.arrow.memory.Accountant.releaseBytes(Accountant.java:221)
	at org.apache.arrow.memory.AllocationManager.release(AllocationManager.java:152)
	at org.apache.arrow.memory.BufferLedger.decrement(BufferLedger.java:157)
	at org.apache.arrow.memory.BufferLedger.release(BufferLedger.java:124)
	at org.apache.arrow.memory.BufferLedger.release(BufferLedger.java:104)
	at org.apache.arrow.vector.BaseValueVector.releaseBuffer(BaseValueVector.java:114)
	at org.apache.arrow.vector.BaseFixedWidthVector.clear(BaseFixedWidthVector.java:248)
	at org.apache.arrow.vector.BaseFixedWidthVector.close(BaseFixedWidthVector.java:238)
	at Test.main(Test.java:46)

version:

12.0.0

Component(s)

Java

@lidavidm
Copy link
Member

lidavidm commented Jun 8, 2023

Thanks! It seems likely there's an underflow or overflow here. (I'm curious why it lets you allocate from a in the first place, so I'll dig into that too; there's an "overlimit" situation but AFAIK it only applies when transferring buffers between allocators)

lidavidm added a commit to lidavidm/arrow that referenced this issue Jun 20, 2023
lidavidm added a commit that referenced this issue Jun 29, 2023
### Rationale for this change

The Java allocator wasn't detecting overflow (which is admittedly a corner case since you can't actually allocate that many bytes), causing an unexpected exception.

### What changes are included in this PR?

Detect overflow and provide the right exception.

### Are these changes tested?

Yes

### Are there any user-facing changes?

No
* Closes: #35960

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 13.0.0 milestone Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants