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

Fix unexpected java.lang.ArithmeticException when converting Rational to BigDecimal #4336

Merged
merged 1 commit into from Dec 5, 2016

Conversation

kirs
Copy link
Contributor

@kirs kirs commented Nov 26, 2016

I'm not sure that catch (ArithmeticException e) is the best way to fix it, but at least this would emulate MRI behaviour (read #4324 for more context)

Fixes #4324

@enebo @headius

when converting Rational to BigDecimal
Fixes jruby#4324
@olleolleolle
Copy link
Member

olleolleolle commented Nov 29, 2016

(Totally unrelated to your fix:) The appveyor CI job failed with this message: https://ci.appveyor.com/project/jnr/jruby/build/2880/job/shniu1cn7yluu99o#L1692

Java HotSpot(TM) Client VM warning: ignoring option MaxPermSize=2G;
  support was removed in 8.0
Error occurred during initialization of VM
Could not reserve enough space for 3145728KB object heap

Hm, perhaps MAVEN_OPTS -Xmx2G could be something?

@kirs
Copy link
Contributor Author

kirs commented Nov 29, 2016

@olleolleolle I think that the ignoring option is just a warning because it's also printed on successful test runs:

Java HotSpot(TM) 64-Bit Server VM warning: ignoring option MaxPermSize=2G; support was removed in 8.0
Running org.jruby.embed.internal.BiVariableMapTest
Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 10.485 sec - in 

I'm more worried about other failures like uninitialized constant FFI::Platform. Perhaps we should create a separate issue to discuss ways how we can fix Appveyor.

@kirs
Copy link
Contributor Author

kirs commented Nov 30, 2016

@enebo @headius you guys have any thoughts on this fix?

@kares
Copy link
Member

kares commented Dec 5, 2016

@kirs najs fix ... although it looks a bit scary on first sight :)
the docs on java.math.BigDecimal#divide says throws ArithmeticException - if the exact quotient does not have a terminating decimal expansion ... so this should be very decent.

@kares kares added this to the JRuby 9.1.7.0 milestone Dec 5, 2016
@enebo
Copy link
Member

enebo commented Dec 5, 2016

@kares if you reviewed this and are happy with it you can merge it. I felt out of depth on this one.

@kares kares merged commit ec525af into jruby:master Dec 5, 2016
@kirs kirs deleted the rational-to-bigdecimal-fix branch December 5, 2016 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crashing on attempt to convert Rational into BigDecimal
4 participants