Skip to content

Handle possible ArithmeticException when extending byte store for String#6670

Merged
headius merged 1 commit intojruby:masterfrom
xaptronic:negative_array_size_exception
May 12, 2021
Merged

Handle possible ArithmeticException when extending byte store for String#6670
headius merged 1 commit intojruby:masterfrom
xaptronic:negative_array_size_exception

Conversation

@xaptronic
Copy link
Contributor

I have a JRuby application that writes data into a StringIO. After a certain amount of data, the application throws a long winded exception with stack trace leading to ByteList::ensure with exception Java::JavaLang::NegativeArraySizeException.

It appears that the code attempts to extend the byte arrray beyond Integer.MAX_VALUE length which results in attempting to create a byte[] of negative length.

Relevant portion of stacktrace:

Java::JavaLang::NegativeArraySizeException:
--
org.jruby.util.ByteList.ensure(ByteList.java:344)
org.jruby.util.io.EncodingUtils.strBufCat(EncodingUtils.java:1712)
org.jruby.util.io.EncodingUtils.strBufCat(EncodingUtils.java:1697)
org.jruby.util.io.EncodingUtils.encCrStrBufCat(EncodingUtils.java:1820)
org.jruby.util.io.EncodingUtils.encStrBufCat(EncodingUtils.java:1718)
org.jruby.ext.stringio.StringIO.stringIOWrite(StringIO.java:1209)
org.jruby.ext.stringio.StringIO.write(StringIO.java:1170)
org.jruby.ext.stringio.StringIO$INVOKER$i$write.call(StringIO$INVOKER$i$write.gen)

This PR wraps the calculation in a try/catch and uses Math.addExact to catch a potential ArithmeticException. In the even that is does, we set the new allocation size to Integer.MAX_VALUE.

Retesting, I now get a more useful exception: Java::JavaLang::OutOfMemoryError: Java heap space.

…h ArithmeticException and extend to Integer.MAX_VALUE if caught
@headius
Copy link
Member

headius commented May 12, 2021

Thanks for the PR! I will review.

@headius
Copy link
Member

headius commented May 12, 2021

Looks good! I am playing with numbers around the threshold now and seeing the error exactly when the string size goes to MAX_VALUE - 1... not when it tries to go above that. I believe the JVM error is being raised for allocating an array at that threshold, so the effective size is really MAX_VALUE - 2 then? Just wondering if that should be the value we attempt to use so we get every last byte of space.

[] ~/projects/jruby $ jruby -e 'foo = "x" * 2147483647; loop { p "loop"; foo << "x" }'
Error: Your application used more memory than the automatic cap of 4096MB.
Specify -J-Xmx####M to increase it (#### = cap size in MB).
Specify -w for full java.lang.OutOfMemoryError: Requested array size exceeds VM limit stack trace

[] ~/projects/jruby $ jruby -e 'foo = "x" * 2147483646; loop { p "loop"; foo << "x" }'
Error: Your application used more memory than the automatic cap of 4096MB.
Specify -J-Xmx####M to increase it (#### = cap size in MB).
Specify -w for full java.lang.OutOfMemoryError: Requested array size exceeds VM limit stack trace

[] ~/projects/jruby $ jruby -e 'foo = "x" * 2147483645; loop { p "loop"; foo << "x" }'
"loop"
Error: Your application used more memory than the automatic cap of 4096MB.
Specify -J-Xmx####M to increase it (#### = cap size in MB).
Specify -w for full java.lang.OutOfMemoryError: Requested array size exceeds VM limit stack trace

Notice only the MAX_VALUE - 2 is able to allocate the multiplied string, and it cannot be coaxed into adding one more byte.

I think this is a case where we should also catch the OutOfMemorError to raise something a bit more Ruby... but do it carefully so we aren't masking a real heap exhaustion error. This is not for you to fix, I just mention it here before merging.

Thank you!

@headius
Copy link
Member

headius commented May 12, 2021

So step debugging through the String#* logic (which does not use the path you patched) I came to this point in the debugger:

Screen Shot 2021-05-12 at 13 16 11

The next step fails with the array-size heap error. It seems that the effective size of a Java byte[] is actually MAX_VALUE - 2 so perhaps that should be the fallback max size we use.

I will merge your PR and then open another to audit all places we allocate arrays in this way.

@headius
Copy link
Member

headius commented May 12, 2021

I will merge this to master but then cherry-pick the change into 9.2 and do additional auditing and fixes there. We plan to maintain 9.2.x at least through the end of the year.

@headius headius merged commit 460bb69 into jruby:master May 12, 2021
@headius headius added this to the JRuby 9.2.18.0 milestone May 12, 2021
@xaptronic
Copy link
Contributor Author

Did you change the patch to be MAX_VALUE - 2?

headius added a commit to headius/jruby that referenced this pull request May 12, 2021
This also reduces the fallback maximum to Integer.MAX_VALUE - 2,
which seems to be the actual effective size for allocating a new
array. See jruby#6670.
@headius
Copy link
Member

headius commented May 12, 2021

@xaptronic I have started working on additional fixes, including the MAX - 2 change, in #6671.

headius added a commit to headius/jruby that referenced this pull request May 12, 2021
Based on explorations for jruby#6670 we found that the effective max
array size is actually Integer.MAX_VALUE - 2, so this localizes
the multiplication logic and uses that limit as the upper bound.

$ jruby -w -e 'foo = "x" * 2147483645; p :ok'
:ok

$ jruby -w -e 'foo = "x" * 2147483646; p :ok'
ArgumentError: argument too big
       * at org/jruby/RubyString.java:1197
  <main> at -e:1
headius added a commit to headius/jruby that referenced this pull request May 12, 2021
Based on explorations for jruby#6670 we found that the effective max
array size is actually Integer.MAX_VALUE - 2, so this localizes
the multiplication logic and uses that limit as the upper bound.

$ jruby -w -e 'foo = "x" * 2147483645; p :ok'
:ok

$ jruby -w -e 'foo = "x" * 2147483646; p :ok'
ArgumentError: argument too big
       * at org/jruby/RubyString.java:1197
  <main> at -e:1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants