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

RxTextView.textChanges crashes when one of subscriptions produces an error #192

Open
netimen opened this issue Nov 12, 2015 · 7 comments
Open

Comments

@netimen
Copy link

netimen commented Nov 12, 2015

Hi Jake and everyone! Thanks for excellent job!

When I use RxTextView.textChanges with several subscriptions and one of them produces an error (which I handle properly) the app crashes.

Here is the simple code:

RxTextView.textChanges(editQuery)
        .skip(1) // we need the error to happen when the user changes the text so skipping initial value
        .concatMap(cs -> Observable.error(new Exception("aaa")))
        .subscribe(Observers.create(o -> {}, e -> Log.e(LOG_TAG, "error" + e)));

RxTextView.textChanges(editQuery)
        .subscribe(s -> Log.e(LOG_TAG, "text: " + s));

When I run my app and type something in my EditText the app crashes:

java.lang.IndexOutOfBoundsException: Invalid index 1, size is 1
     at java.util.ArrayList.throwIndexOutOfBoundsException(ArrayList.java:255)
     at java.util.ArrayList.get(ArrayList.java:308)
     at android.widget.TextView.sendOnTextChanged(TextView.java:7679)
     at android.widget.TextView.handleTextChanged(TextView.java:7739)
     at android.widget.TextView$ChangeWatcher.onTextChanged(TextView.java:9472)
     at android.text.SpannableStringBuilder.sendTextChanged(SpannableStringBuilder.java:964)
     at android.text.SpannableStringBuilder.replace(SpannableStringBuilder.java:515)
     at android.text.SpannableStringBuilder.replace(SpannableStringBuilder.java:454)
     at android.text.SpannableStringBuilder.replace(SpannableStringBuilder.java:33)
     at android.view.inputmethod.BaseInputConnection.replaceText(BaseInputConnection.java:685)
     at android.view.inputmethod.BaseInputConnection.setComposingText(BaseInputConnection.java:445)
     at com.android.internal.view.IInputConnectionWrapper.executeMessage(IInputConnectionWrapper.java:340)
     at com.android.internal.view.IInputConnectionWrapper$MyHandler.handleMessage(IInputConnectionWrapper.java:78)
     at android.os.Handler.dispatchMessage(Handler.java:102)
     at android.os.Looper.loop(Looper.java:135)
     at android.app.ActivityThread.main(ActivityThread.java:5257)
     at java.lang.reflect.Method.invoke(Native Method)
     at java.lang.reflect.Method.invoke(java.lang.reflect.Method.java:372)
     at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)

And it happens because when user types something, the first subscription throws an Exception and then TextView.removeTextChangedListener is called in TextViewTextOnSubscribe.java:39 (because of unsubscribe after error). But the TextView is still iterating over it's watchers in the TextView.sendOnTextChanged method!

@dlew
Copy link
Contributor

dlew commented Nov 12, 2015

Interesting; this seems to be a concurrency issue in the Android listener itself. Having solved an almost identical problem to this recently, it's only really solvable if the emitter uses something like CopyOnWriteArrayList (or equivalent concurrency structure) for the listener list.

A simple workaround would be to share the Observable from textChanges() (via something like publish()). Not sure what RxBinding could do to fix it besides warn users of the dangers here.

@dlew
Copy link
Contributor

dlew commented Nov 12, 2015

(In case anyone is curious, here's the exact same problem in a slightly different context and the fix I settled on for now.)

@JakeWharton
Copy link
Owner

I believe you could also add .unsubscribeOn(mainThread()) which would delay
unsubscription with a post to the next frame.

On Thu, Nov 12, 2015 at 9:04 AM Daniel Lew notifications@github.com wrote:

(In case anyone is curious, here's the exact same problem in a slightly
different context trello-archive/navi#15 and the fix
I settled on for now
trello-archive/navi@c541812
.)


Reply to this email directly or view it on GitHub
#192 (comment)
.

@micdm
Copy link

micdm commented Jun 24, 2016

Have spent several hours to locate that bug. The easiest example to reproduce (place it to Activity.onCreate for example):

TextView view = new TextView(this);
RxTextView.textChanges(view).skip(1).take(1).subscribe();
RxTextView.textChanges(view).skip(1).subscribe();
view.setText("hello");

Workaround may be (as @dlew mentioned):

Observable<CharSequence> text = RxTextView.textChanges(view).skip(1).share();
text.take(1).subscribe();
text.subscribe();
view.setText("hello");

@JakeWharton
Copy link
Owner

Has a bug been filed on b.android.com for this ever? I'm struggling to have any desire to fix this in the library.

@micdm
Copy link

micdm commented Jun 24, 2016

https://code.google.com/p/android/issues/detail?id=190399 is about that. Seems pretty dead.

@JakeWharton
Copy link
Owner

Great! I'll take care of it.

On Fri, Jun 24, 2016, 1:40 AM Mikhail Demerzov notifications@github.com
wrote:

https://code.google.com/p/android/issues/detail?id=190399 is about that.
Seems pretty dead.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#192 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAEEEfZm7o_dxSM7r6sUOQ2MeYTRshM6ks5qO226gaJpZM4Gg_co
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants