Scala Programming Language
  1. Scala Programming Language
  2. SI-6173

BigDecimal#isWhole implementation is very heap intensive

    Details

      Description

      {{

      { BigDecimal("-4.6010652208491219E+1556245283").hashCode }

      }}

      Explodes with an OutOfMemoryError even with a 4GB heap. (tested using PaulP's sbt-extras script with SBT_MEM=4096) The issue is that the BigDecimal#isWhole function is extremely heap-intensive for large numbers. This is actually a bit silly, given that numbers like the one I pasted are never, ever going to be convertible to either Int or Double, which is what the isWhole check is intended to accomplish.

      My proposal would be to change the implementation of BigDecimal#hashCode to do a quick sanity check on the size of the value to see if it's even possible to fall into the range of Int/Double. If not, then check the sign and return the value that Double's ## would have returned anyway (Double.POSITIVE_INFINITY or Double.NEGATIVE_INFINITY). As things stand, this is literally a security whole in Scala for people who parse user-specified text as BigDecimal, even with non-ARBITRARY contexts.

        Activity

        Hide
        Paul Phillips added a comment -

        The rounding issue is our fault - that's why they become equal when I pass a java.math.BigDecimal to scala.BigDecimal, but not if I create a scala.BigDecimal from the same String.

        Show
        Paul Phillips added a comment - The rounding issue is our fault - that's why they become equal when I pass a java.math.BigDecimal to scala.BigDecimal, but not if I create a scala.BigDecimal from the same String.
        Hide
        Jeff Skaistis added a comment -

        Comment from the Java BigDecimal source for bigTenToThe():

        // BigInteger.pow is slow, so make 10**n by constructing a
        // BigInteger from a character string (still not very fast)

        Unfortunately, there's not a limit check in there, so it's creating a 3 GB array and dumping 1.5 billion zeros into it. Of course the actual pow() method does have a sanity check, so it would have been better just to call it.

        I think the lesson here is that BigDecimal shouldn't be treated like it was an actual floating-point number. Personally, I'd like to see support for the new IEEE decimal floating points.

        Show
        Jeff Skaistis added a comment - Comment from the Java BigDecimal source for bigTenToThe(): // BigInteger.pow is slow, so make 10**n by constructing a // BigInteger from a character string (still not very fast) Unfortunately, there's not a limit check in there, so it's creating a 3 GB array and dumping 1.5 billion zeros into it. Of course the actual pow() method does have a sanity check, so it would have been better just to call it. I think the lesson here is that BigDecimal shouldn't be treated like it was an actual floating-point number. Personally, I'd like to see support for the new IEEE decimal floating points.
        Hide
        Simon Ochsenreither added a comment -

        Java 8 ships with a considerably improved version of BigInteger. Might make sense to check whether and how some issues might have been solved on their side.

        Show
        Simon Ochsenreither added a comment - Java 8 ships with a considerably improved version of BigInteger. Might make sense to check whether and how some issues might have been solved on their side.
        Hide
        Rex Kerr added a comment -

        We won't support Java 8 exclusively for quite some time, so an easy fix shouldn't be delayed that long. There is an "easy" fix that I will supply.

        Show
        Rex Kerr added a comment - We won't support Java 8 exclusively for quite some time, so an easy fix shouldn't be delayed that long. There is an "easy" fix that I will supply.
        Show
        Adriaan Moors added a comment - https://github.com/scala/scala/pull/3316

          People

          • Assignee:
            Rex Kerr
            Reporter:
            Daniel Spiewak
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development