Multiple push messages: The content of the adapter has changed but ListView did not receive a notification

Question

When I receive a lot of push messages (let's say 50) from GCM within 1 second I receive the following exception:

java.lang.IllegalStateException: The content of the adapter has changed but ListView did not receive a notification. Make sure the content of your adapter is not modified from a background thread, but only from the UI thread. [in ListView(2131427434, class android.widget.ListView) with Adapter(class a.n)] at android.widget.ListView.layoutChildren(ListView.java:1544) at android.widget.AbsListView.onLayout(AbsListView.java:2045) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1670) at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1528) at android.widget.LinearLayout.onLayout(LinearLayout.java:1441) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.support.v4.view.ViewPager.onLayout(Unknown Source) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1670) at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1528) at android.widget.LinearLayout.onLayout(LinearLayout.java:1441) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.support.v4.widget.DrawerLayout.onLayout(Unknown Source) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.widget.FrameLayout.onLayout(FrameLayout.java:446) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.support.v7.internal.widget.ActionBarOverlayLayout.onLayout(Unknown Source) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.widget.FrameLayout.onLayout(FrameLayout.java:446) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1670) at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1528) at android.widget.LinearLayout.onLayout(LinearLayout.java:1441) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.widget.FrameLayout.onLayout(FrameLayout.java:446) at android.view.View.layout(View.java:14255) at android.view.ViewGroup.layout(ViewGroup.java:4413) at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:1998) at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1812) at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1050) at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:4560) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:749) at android.view.Choreographer.doCallbacks(Choreographer.java:562) at android.view.Choreographer.doFrame(Choreographer.java:532) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:735) at android.os.Handler.handleCallback(Handler.java:725) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:137) at android.app.ActivityThread.main(ActivityThread.java:5171) 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:797) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:564) at dalvik.system.NativeStart.main(Native Method)

I have already tried to fix this, by putting BOTH the messages.add() and notifyDataSetChanged() inside the runOnUIThread. I guess that happens because onUpdate() of my listener is called for every push message. But shouldn't that problem be solved by runOnUIThread(), because everything is executed consecutively?

    MainApplication app = (MainApplication) context.getApplicationContext();
    app.setOnRoomMessageUpdateListener(new OnRoomMessageUpdateListener() {
        @Override
        public void onUpdate() {
            // save message with highest time, so we can only query the new
            // messages
            long highestTime = getHighestMessageTime();

            messageDatabase.getConditionBuilder().add(
                    DatabaseHelper.KEY_MESSAGE_ROOM_ID + " = ? AND " + DatabaseHelper.KEY_MESSAGE_LOCAL_TIME
                            + " > ? AND " + DatabaseHelper.MESSAGE_TABLE_NAME + "."
                            + DatabaseHelper.KEY_MESSAGE_USER_ID + " <> ?",
                    new String[] { String.valueOf(roomID), String.valueOf(highestTime),
                            String.valueOf(user.getUserID()) });
            messageDatabase.getConditionBuilder().setSortOrder(DatabaseHelper.KEY_MESSAGE_LOCAL_TIME + " DESC");
            final ArrayList<Message> newMessages = messageDatabase.getList();

            ((Activity) context).runOnUiThread(new Runnable() {
                @Override
                public void run() {
                    messages.addAll(newMessages);
                    messageAdapter.notifyDataSetChanged();
                }
            });
        }
    });

EDIT: I probably missed out a very important part of the code, which I overlooked myself:

app.setOnRoomUserUpdateListener(new OnRoomUserUpdateListener() {
    @Override
    public void onUpdate(final User user, final int roomID, final int joinStatus) {
        final String message;
        if (joinStatus == OnRoomUserUpdateListener.USER_JOINED) {
            message = context.getString(R.string.join_room_message, user.getUsername());
        } else {
            message = context.getString(R.string.leave_room_message, user.getUsername());
        }

        ((Activity) context).runOnUiThread(new Runnable() {
            @Override
            public void run() {
                messages.add(new Message(-1, user, message, System.currentTimeMillis(), System
                        .currentTimeMillis(), false, roomID, 0, true, Message.TYPE_JOINLEAVE));
                messageAdapter.notifyDataSetChanged();
            }
        });
    }
});

This code block is located below the code above and it's obviously also modifying the content of the adapter. When both of them run simultaneous, there could be a problem, right? Could this be fixed by using synchronized or is there a better way?

EDIT 2: Initialization:

// get all messages
    messages = new ArrayList<Message>();
    messageDatabase.getConditionBuilder().add(DatabaseHelper.KEY_MESSAGE_ROOM_ID + " = ?",
            new String[] { String.valueOf(roomID) });
    messageDatabase.getConditionBuilder().setSortOrder(DatabaseHelper.KEY_MESSAGE_LOCAL_TIME + " DESC");
    messageDatabase.getConditionBuilder().setSqlLimit(100);
    messages.addAll(messageDatabase.getList());

    // get "user joined/left" messages
    UserDatabase userDatabase = UserDatabase.getInstance(context);
    messages.addAll(userDatabase.getJoinLeaveMessages(roomID));

    Collections.sort(messages);
    messageAdapter = new MessageAdapter(getActivity(), R.layout.list_message_item, messages);
    listView.setAdapter(messageAdapter);

Edit 3: Full source of the fragment, which contains the code: https://gist.github.com/ChristopherWalz/89a071b1606460e18ce7

Boathouse

Could you please post a full stack trace of the exception so that we know which parts of AOSP code are relevant? Also, it looks suspicious that you add new messages with messages.add() (as opposed to messageAdapter.add()) - modifying Adapter's data externally to the Adapter is a bad practice. Are there any other places in the code where messages gets modified? – Darell

Updated with full Stacktrace. messages.add() is called three times: Two parts are posted in my question, the other part is when writing a message on your own. But this should not be relevant, because the exception is happening when turning of the internet, received ~50 push messages and turning the internet on again, so all the push messages are received within 1 second. – Boathouse

I was once trying to do something similar where my adapter needed to be refreshed at this rate. I tried to synchronize but even then there used to be inconsistency in the data. So, I think the best way is to reduce the frequency of your adapter update. – Alfano

I tried the exact same thing that @daniel-nugent suggested in his answer. – Alfano

And did it work using synchronized? How do other apps which receive a lot of push messages handle them? I guess I'm not the only one with this kind of problem. – Boathouse

how did you link you messages to your messageAdapter? – Archbishopric

Answer 1

First of all, let's take a look at the code in ListView that throws this exception:

@Override
protected void layoutChildren() {
    // ... code omitted...

        // Handle the empty set by removing all views that are visible
        // and calling it a day
        if (mItemCount == 0) {
            resetList();
            invokeOnItemScrollListener();
            return;
        } else if (mItemCount != mAdapter.getCount()) {
            throw new IllegalStateException("The content of the adapter has changed but "
                    + "ListView did not receive a notification. Make sure the content of "
                    + "your adapter is not modified from a background thread, but only from "
                    + "the UI thread. Make sure your adapter calls notifyDataSetChanged() "
                    + "when its content changes. [in ListView(" + getId() + ", " + getClass()
                    + ") with Adapter(" + mAdapter.getClass() + ")]");
        }
    // ... code omitted...
}

[you might notice that the message is different, but it is just because you're using old Android - the message has been made more informative in September `13]

Let's see where mItemCount member variable is declared... Hmm, this variable seems to be inherited all the way from the AdapterView class. Ok, let's find all the assignments in all the classes:

Basically, except for a single assignment to 0 (which is in onIvalidated() method and can be ignored), this variable is always assigned to Adapter's getCount() value.

We can conclude that you get the exception because there is some concurrent code which alters your Adapter's data while it is used to draw the contents of the ListView.

Now, from your question it looks like you suspect that there is some kind of "congestion" of updates on the UI thread because there are too many messages... However, let's keep in mind that the code of Runnable.run() which you post to UI thread for execution executes atomically - each Runnable is popped from the UI thread's event queue and is run to completion before any other event gets a chance to be processed.

The above means that messages will be updated with a new data and messageAdapter will handle the change immediately, and no other event can interfere with this flow (as long as messages.add() and messages.addAll() in your code are synchronous calls). Bottom line: the code that dispatches Runnables to the UI thread looks fine, and it is unlikely that it is the source of the problem. Furthermore, the stack trace of the exception does not contain any reference to Adapter.

Up until now we summarized facts. Let's start the guesswork.

I think that the issue is not in the code you've posted. I guess you do one (or more) of the following actions, each of which can lead to the exception you got:

1. I guess that messages is the same data structure used by messageAdapter internally. It might be the case that you modify messages in some other part of the code, which is not running on UI thread. In this case this modification can happen while the ListView is refreshed on the UI thread and lead to the exception [it is generally bad practice to "leak" Adapter's data structure outside of the Adapter object].
2. Similarly, you might be accidentally manipulating messageAdapter in other parts of the code not running on UI thread.
3. It might be the case that you post events to UI thread while ListView has not completed its initialization yet. I hardly believe that this is the case, but in order to be on the safe side, I'd suggest you ensure that you register your listeners in onResume() and unregister them in onPause()

EDIT:

Based on the code of your Fragment I can see two possible causes for the exception:

1. If the methods of CustomResponseHandler you pass to ServerUtil.post() will be called from background threads, then the fact that you remove object from messages in onFailure() may be the case.
2. As I suggested - register listeners in onResume() and unregister in onPause() - this might not be important for button click listeners, but it causes memory leaks when you pass these listeners to Application object. You have memory leaks in your code.

If none of the above helps, post the code of MessageAdapter and MessageDatabase

Darell

Please check my EDIT 3, I've added the complete code of the fragment. There are some parts, where messages/messageAdapter get touched, but I thought they are not that important. – Boathouse

@vasiliy I agree with your comment. I see now that the solution proposed in my answer would not have fixed the issue. Your answer is much better than the one I posted, so I deleted mine, and upvoted yours. Nice answer! – Fetish

Okay, so I've moved the messages.remove() parts to the UI Thread and register and unregister the listener in onResume/onPause. I will try it tomorrow and give you some feedback. – Boathouse

BTW: "[it is generally bad practice to "leak" Adapter's data structure outside of the Adapter object]." - what do you mean by that and could you post an example how to do it correct? – Boathouse

@Chris, "leak" adapter data structure = keep the reference to messages outside of messageAdapter and use it to change adapter's data. In general, you should change the data bound by the adapter only through adapter's methods (e.g. messageAdapter.add(), nessageAdapter.remove(), etc) – Darell

So I should not declare messages as a member variable of the fragment instead just pass the initial messages to the adapter and then use messageAdapter.add () and messageAdapter.remove () while messageAdapter is a member variable of the fragment, correct? – Boathouse

@Chris, yes, this is the preferred approach which is less bugs prone – Darell

Okay, but then I'm facing multiple problems: 1. Iterating through messages is no more possible (for example needed in getHighestMessageTime). I would have to implement a method getItems in my custom adapter and return items. 2. This method would not always return the current items, for example after I added a new one. Because I call super(context, resource, items); in my adapter, which passes my items by value to mObjects. After using adapter.add(), mObjects would be updated, but getItems() would return an old list. How to fix these problems with a proper implementation? – Boathouse

@Chris, your original question concerns a particular issue. I think that the answer above is enough to address these issues. If you want to get more information about working with adapters, I suggest you ask additional question with appropriate title – Darell

Answer 2

1. Create a instance of adapter once while starting activity or fragment something like this:

messageAdapter = new MessageAdapter(getActivity(), R.layout.list_message_item, messages); listView.setAdapter(messageAdapter);

Then write your message adapter class

Message Adapter Class:

private ArrayList<Message> mMessageList;

 // constructor
MessageAdapter(Activity activity,int layoutId, ArrayList<Message> messageList){

..
  updateMessageList(messageList);

}
public void updateMessageList(ArrayList<Message> newMessageList){

      // if First time this method is called then 
      if(mMessageList==null){
         mMessageList=new ArrayList<Message>();
      } 

      //Add new messages to message list
      if(null!=newMessageList && newMessageList.size()>0){

         //add new messages
          mMessageList.addAll(newMessageList);

          //Sort you message list
           Collections.sort(mMessageList);
      }
      //update list
      notifyDataSetChanged();

}
.....
end adapter class

3. Now From Activity Class or Fragment class You can use a handler since it kind of act as a bridge between two threads(UI to background thread also) something like this.

      //local message list declaration
       messages=new ArrayList<Message>();
      //Declare handler as a Class member variable 
       Handler globalhandler=new Handler() {
        @Override
        public void handleMessage(Message msg) {
            if (msg.what == "Some identifier string") {

               // after 50 messages are collected it sends one update
    //request after one second to list adapter
                   messageAdapter.updateMessageList(messages);

    // you can also fetch messages from db and update list by calling messageAdapter.updateMessageList(messages); method

                }

             }
           }


app.setOnRoomUserUpdateListener(new OnRoomUserUpdateListener() {
    @Override
    public void onUpdate(final User user, final int roomID, final int joinStatus) {

           final String message;
    if (joinStatus == OnRoomUserUpdateListener.USER_JOINED) {
        message = context.getString(R.string.join_room_message, user.getUsername());
    } else {
        message = context.getString(R.string.leave_room_message, user.getUsername());
    }
          messages.add(new Message(-1, user, message, System.currentTimeMillis(), System
                        .currentTimeMillis(), false, roomID, 0, true, Message.TYPE_JOINLEAVE));           


        //post via handler you can also send a delayed update(like 1 second) since you are receiving 50 messages in a second.
        globalhandler.removeMessages("Some identifier string");
        globalhandler.sendEmptyMessageDelayed("Some identifier string", 1000);
});

I hope this answers your question.

Frog