817386 - java.lang.IndexOutOfBoundsException: getChars (0 ... <n>) ends beyond length 0 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java) at org.mozilla.gecko.GeckoEditable.invoke(GeckoEditable.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[Scoobidiver (away)]

This bug was filed from the Socorro interface and is 
report bp-cc77588f-7669-4a57-bc51-2c6822121202 .
=============================================================

Comment 1

[Scoobidiver (away)]

With that stack trace, there are three crashes in 20.0a1, the first one in 20.0a1/20121126.

java.lang.IndexOutOfBoundsException: getChars (0 ... 13) ends beyond length 0
	at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java:1016)
	at android.text.SpannableStringBuilder.getChars(SpannableStringBuilder.java:913)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at org.mozilla.gecko.GeckoEditable.invoke(GeckoEditable.java:636)
	at $Proxy0.getChars(Native Method)
	at android.text.TextUtils.getChars(TextUtils.java:70)
	at android.text.TextUtils.substring(TextUtils.java:256)
	at android.view.inputmethod.BaseInputConnection.getTextBeforeCursor(BaseInputConnection.java:335)
	at com.android.internal.view.IInputConnectionWrapper.executeMessage(IInputConnectionWrapper.java:218)
	at com.android.internal.view.IInputConnectionWrapper$MyHandler.handleMessage(IInputConnectionWrapper.java:77)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4898)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1006)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:773)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/query/query?product=FennecAndroid&version=FennecAndroid%3A20.0a1&range_value=4&range_unit=weeks&query_search=signature&query_type=contains&query=android.text.SpannableStringBuilder.checkRange&do_query=1

Comment 2

[Scoobidiver (away)]

Aurora seems unaffected.
There are 6 crashes in Nightly making it #11-19 top crasher.

Comment 3

[Scoobidiver (away)]

It's now #9 top crasher in 20.0a1.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking since it's a topcrasher, we can check back on the volume when we go to Aurora.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Log and ignore Editable exceptions (v1)

This happens because of a race between Gecko changing the text and InputConnection reading the text. InputConnection first reads the selection offsets; Gecko then changes the text which alters the selection; InputConnection finally uses the old selection offsets to try to get text, and in this case the old selection offsets are outside of the text.

It will take a lot of work to fix this race, because when Gecko single-handedly changes the text, Java cannot simply wait for Gecko. There are several approaches, but the simplest case is probably just to catch the exception, and make Fennec not crash. The IME might misbehave as a result but this race should not be a common case, and I think it's a good compromise.

Comment 6

[Chris Peterson [:cpeterson]]

Comment on attachment 698814 [details] [diff] [review]
Log and ignore Editable exceptions (v1)

Review of attachment 698814 [details] [diff] [review]:
-----------------------------------------------------------------

This fix is kinda ugly, but I realize this is a workaround for a topcrash. These race conditions between the UI and Gecko threads have been a long sore point; it would be nice to fix a robust solution.

To avoid masking other bugs that might be throwing exceptions, can we only handle just the IndexOutOfBoundsException?

::: mobile/android/base/GeckoEditable.java
@@ +741,5 @@
> +            // return an empty value instead.
> +            Log.w(LOGTAG, "Exception in GeckoEditable." + method.getName(), e.getCause());
> +            Class<?> retClass = method.getReturnType();
> +            ret = retClass != Void.TYPE && retClass.isPrimitive() ? retClass.newInstance() :
> +                  retClass == String.class ? "" : null;

I'm a big fan of the ?: ternary operator, but I think nesting two ternary expressions together is too clever. :)

Let's just split this into something boring like:

    if (retClass != Void.TYPE && retClass.isPrimitive())
        ret = retClass.newInstance();
    else if (retClass == String.class)
        ret = "";
    else
        ret = null;

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/efddd115fd76

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Log and ignore IndexOutOfBoundsException in Editable (v1.1)

Patch after addressing review comments.

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 699885 [details] [diff] [review]
Log and ignore IndexOutOfBoundsException in Editable (v1.1)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Top-crash that's being tracked for Aurora
Testing completed (on m-c, etc.): Locally
Risk to taking this patch (and alternatives if risky): No risk; patch only catches crash and prevents crash from happening
String or UUID changes made by this patch: None

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/efddd115fd76

Comment 11

[bhavana bajaj [:bajaj]]

Comment on attachment 699885 [details] [diff] [review]
Log and ignore IndexOutOfBoundsException in Editable (v1.1)

Approving on aurora.
Please keep an eye on the crash volume to make sure the patch has helped & lookout for any new regressions.

Comment 12

[Scoobidiver (away)]

It's not fixed: https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.GeckoEditable.replace%28GeckoEditable.java%29
but no longer a top crasher

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Scoobidiver from comment #12)
> It's not fixed:
> https://crash-stats.mozilla.com/report/list?signature=java.lang.
> NullPointerException%3A+at+org.mozilla.gecko.GeckoEditable.
> replace%28GeckoEditable.java%29
> but no longer a top crasher

The stack is very different. It should be a different bug.

Comment 14

[Scoobidiver (away)]

(In reply to Jim Chen [:jchen :nchen] from comment #13)
> The stack is very different. It should be a different bug.
OK. I will file it.

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/006d27ddd615