Fixed concurrent modifications in UI components when subscribing taken events #162
Conversation
| @@ -75,7 +75,7 @@ public boolean removeOnClickListener(final View.OnClickListener listener) { | |||
|
|
|||
| @Override | |||
| public void onClick(final View view) { | |||
| for (final View.OnClickListener listener : listeners) { | |||
| for (final View.OnClickListener listener: new ArrayList<View.OnClickListener>(listeners)) { | |||
There was a problem hiding this comment.
This is extremely wasteful. Try a CopyOnWriteArrayList for listeners instead?
There was a problem hiding this comment.
Thanks for the reasonable comment.
I modified the codes to match it, and pushed them.
|
Did you find out when/how the list of listeners is being modified from a background thread? |
|
I did. In this case, a taken (and completed) observer will try to unsubscribe events on UiThread after invoking onNext(View). So the solution is using CopyOnWriteArrayList or to create new list, I think. |
| @@ -75,7 +76,7 @@ public boolean removeOnClickListener(final View.OnClickListener listener) { | |||
|
|
|||
| @Override | |||
| public void onClick(final View view) { | |||
| for (final View.OnClickListener listener : listeners) { | |||
| for (final View.OnClickListener listener: listeners) { | |||
There was a problem hiding this comment.
Can you revert all of these small whitespace changes, unnecessary usages of final?
There was a problem hiding this comment.
@ronshapiro I see. Minimized diffs.
|
+1 |
|
This is fixed in https://github.com/JakeWharton/RxBinding but per #172 this won't be going into RxAndroid core. We have removed the view binding observables for the next release. |
This fixes ConcurrentModificationException which is thrown by Observables that invoked Observable#take(int).
Relevant issue is #158 .