Search code examples
androidconcurrentmodification

Android: ConcurrentModificationException persisting


I have 2 lists suggestedFriends and allSuggestedFriends.

I use an iterator to run through the first list and remove items from this list. On the way, items should be removed from the second list if it still has items in it. I use a second iterator (in the SafeRemove method) just for removing items from the second list.

Problem: Sometimes I get a ConcurrentModificationException in the line if (allSuggestedFriends.size() > 0) { even though I use iterators for the propose of removing items.

Should I use an iterator to ask for size? I don't think it's possible(?!)

synchronized (suggestedFriends) {
    for (final Iterator<User> suggestedFriendsIterator = suggestedFriends.iterator(); suggestedFriendsIterator.hasNext();) {
        User friend = suggestedFriendsIterator.next();
        if (friend.userId == request.getUserId()) {
            final int index = suggestedFriends.indexOf(friend);
            if (status) {
                ((FragmentActivityExt) context).runOnUiThread(new Runnable() {
                    @Override
                    public void run() {
                        if(suggestedFriends.size() > 0) {
                            suggestedFriendsIterator.remove();
                        }
                        notifyDataSetChanged();
                        if (allSuggestedFriends.size() > 0) {
                            suggestedFriends.add(2, allSuggestedFriends.get(0));
                            SafeRemove(allSuggestedFriends, allSuggestedFriends.get(0));
                            notifyDataSetChanged();
                        }
                        if (suggestedFriends.size() == 1) {
                            // FIXME workaround to fix list height as wrap_content is not supported by RecyclerView at the moment
                            //set to fit 1 element
                            LinearLayout.LayoutParams params = (LinearLayout.LayoutParams) mListView.getLayoutParams();
                            params.height = EndoUtility.dpToPx(context, 70);
                            mListView.setLayoutParams(params);
                        }

                        if (suggestedFriends.size() == 0) {
                            EventBus.getDefault().post(new NoMoreSuggestedFriendsEvent());
                        }
                    }
                });
            } else {
                if (mListView != null) {
                    ((FragmentActivityExt) context).runOnUiThread(new Runnable() {
                        @Override
                        public void run() {
                            Toast.makeText(context, R.string.networkProblemToast, Toast.LENGTH_LONG).show();
                            ((SuggestedFriendView) mListView.getChildAt(index)).reset();
                        }
                    });
                }
            }
            break;
        }
    }
}

The SafeRemove method using a second iterator:

private void SafeRemove(List<User> list, User friend) {
    Iterator<User> iter = list.iterator();

    while (iter.hasNext()) {
        User user = iter.next();

        if (user.userId == friend.userId)
            iter.remove();
    }
}

Update:

Would it be a solution to add a synchronized around the SafeRemove(allSuggestedFriends, allSuggestedFriends.get(0)); line? like that:

...
synchronized (allSuggestedFriends) {
    SafeRemove(allSuggestedFriends, allSuggestedFriends.get(0));
}
...

or better yet, inside the SafeRemove method? Like that:

private void SafeRemove(List<User> list, User friend) {
    synchronized (list) {
        Iterator<User> iter = list.iterator();

        while (iter.hasNext()) {
            User user = iter.next();

            if (user.userId == friend.userId)
                iter.remove();
        }            
    }
}

Solution

  • Well, as my question was marked as potential duplicate, I didn't get any responses what so ever. This is unfortunate since I'm quite certain the case in discussion is different to the suggested link.

    Nevertheless, I didn't use the synchronized implemented keyword (as I myself suggested in my update), but just implemented a parallel counter which is sat to the lists size when the list finished loading, and then decrements when items are removed from the list. This eliminates the allSuggestedFriends.size() check which is where the ConcurrentModificationException occurred.

    ...
    private int allSuggestedFriendsCount;
    ...
    // allSuggestedFriends loads items
    ...
    allSuggestedFriendsCount = allSuggestedFriends.size();
    ...
    
    ...
    if (allSuggestedFriendsCount > 0) {
        suggestedFriends.add(2, allSuggestedFriends.get(0));
        SafeRemove(allSuggestedFriends, allSuggestedFriends.get(0));
        notifyDataSetChanged();
    }
    ...
    
    private void SafeRemove(List<User> list, User friend) {
        Iterator<User> iter = list.iterator();
    
        while (iter.hasNext()) {
            User user = iter.next();
            if (user.userId == friend.userId) {
                iter.remove();
                allSuggestedFriendsCount--;
            }
        }
    }