From 82e5a2f60afd1cc330ccfa0b81b8824964e976c7 Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Mon, 20 Jul 2015 20:08:14 -0700 Subject: Avoid starting long uninterruptible computations Bug: 21957088 Shorten timeouts. We usually do short computations first, so this helps to avoid starting absurdly big ones. Check size of result before converting to decimal. If the decimal conversion is likely to time out anyway, just pretent we timed out. This turns out to avoid a lot of problems, since BigInteger decimal conversion is the clear bottleneck for something like 100000! And its uninterruptible. Remove the timeout in one case in which we had previously forgotten. Check for interrupts in a couple of more places in BoundedRational. One of these caused log(10^100000) to hang for a long time. One or two trivial cleanups in code that was touched anyway. Change-Id: I3494a8ed28f681fb26634ecd90042e2f2a8008a8 --- src/com/android/calculator2/BoundedRational.java | 11 ++++++ src/com/android/calculator2/Evaluator.java | 49 +++++++++++++++++------- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/com/android/calculator2/BoundedRational.java b/src/com/android/calculator2/BoundedRational.java index 70c21af..2b3d1ed 100644 --- a/src/com/android/calculator2/BoundedRational.java +++ b/src/com/android/calculator2/BoundedRational.java @@ -87,6 +87,11 @@ public class BoundedRational { return CR.valueOf(mNum).divide(CR.valueOf(mDen)); } + // Approximate number of bits to left of binary point. + public int wholeNumberBits() { + return mNum.bitLength() - mDen.bitLength(); + } + private boolean tooBig() { if (mDen.equals(BigInteger.ONE)) return false; return (mNum.bitLength() + mDen.bitLength() > MAX_SIZE); @@ -418,6 +423,9 @@ public class BoundedRational { // This algorithm is very naive, but we doubt it matters. long count = 0; while (n.mod(BigInteger.TEN).equals(BigInteger.ZERO)) { + if (Thread.interrupted()) { + throw new CR.AbortedException(); + } n = n.divide(BigInteger.TEN); ++count; } @@ -502,6 +510,9 @@ public class BoundedRational { if (r.mDen.equals(BigInteger.ONE)) return 0; r = r.reduce(); BigInteger den = r.mDen; + if (den.bitLength() > MAX_SIZE) { + return Integer.MAX_VALUE; + } while (!den.testBit(0)) { ++powers_of_two; den = den.shiftRight(1); diff --git a/src/com/android/calculator2/Evaluator.java b/src/com/android/calculator2/Evaluator.java index a97d946..b779a68 100644 --- a/src/com/android/calculator2/Evaluator.java +++ b/src/com/android/calculator2/Evaluator.java @@ -302,7 +302,7 @@ class Evaluator { .show(); } - private final long MAX_TIMEOUT = 60000; + private final long MAX_TIMEOUT = 15000; // Milliseconds. // Longer is unlikely to help unless // we get more heap space. @@ -314,11 +314,12 @@ class Evaluator { // calculator is restarted. // We'll call that a feature; others // might argue it's a bug. - private final long mQuickTimeout = 1500; + private final long QUICK_TIMEOUT = 1000; // Timeout for unrequested, speculative // evaluations, in milliseconds. - // Could be shorter with a faster asin() - // implementation. + private int mMaxResultBits = 120000; // Don't try to display a larger result. + private final int MAX_MAX_RESULT_BITS = 350000; // Long timeout version. + private final int QUICK_MAX_RESULT_BITS = 50000; // Instant result version. private void displayTimeoutMessage() { final AlertDialog.Builder builder = new AlertDialog.Builder(mCalculator) @@ -329,6 +330,7 @@ class Evaluator { new DialogInterface.OnClickListener() { public void onClick(DialogInterface d, int which) { mTimeout = MAX_TIMEOUT; + mMaxResultBits = MAX_MAX_RESULT_BITS; } }); } @@ -368,21 +370,31 @@ class Evaluator { } @Override protected void onPreExecute() { - long timeout = mRequired ? mTimeout : mQuickTimeout; - if (timeout != 0) { - mTimeoutRunnable = new Runnable() { - @Override - public void run() { - handleTimeOut(); - } - }; - mTimeoutHandler.postDelayed(mTimeoutRunnable, timeout); + long timeout = mRequired ? mTimeout : QUICK_TIMEOUT; + mTimeoutRunnable = new Runnable() { + @Override + public void run() { + handleTimeOut(); + } + }; + mTimeoutHandler.postDelayed(mTimeoutRunnable, timeout); + } + private boolean isTooBig(CalculatorExpr.EvalResult res) { + int maxBits = mRequired ? mMaxResultBits : QUICK_MAX_RESULT_BITS; + if (res.mRatVal != null) { + return res.mRatVal.wholeNumberBits() > maxBits; + } else { + return res.mVal.get_appr(maxBits).bitLength() > 2; } } @Override protected InitialResult doInBackground(Void... nothing) { try { CalculatorExpr.EvalResult res = mExpr.eval(mDm); + if (isTooBig(res)) { + // Avoid starting a long uninterruptible decimal conversion. + return new InitialResult(R.string.timeout); + } int prec = INIT_PREC; String initCache = res.mVal.toString(prec); int msd = getMsdPos(initCache); @@ -422,7 +434,14 @@ class Evaluator { mEvaluator = null; mTimeoutHandler.removeCallbacks(mTimeoutRunnable); if (result.isError()) { - mCalculator.onError(result.mErrorResourceId); + if (result.mErrorResourceId == R.string.timeout) { + if (mRequired) { + displayTimeoutMessage(); + } + mCalculator.onCancelled(); + } else { + mCalculator.onError(result.mErrorResourceId); + } return; } mVal = result.mVal; @@ -453,6 +472,8 @@ class Evaluator { } @Override protected void onCancelled(InitialResult result) { + // Invoker resets mEvaluator. + mTimeoutHandler.removeCallbacks(mTimeoutRunnable); if (mRequired && !mQuiet) { displayCancelledMessage(); } // Otherwise timeout processing displayed message. -- cgit v1.2.3