ConcurrentModificationException with incorrect line number in stack trace

Android
1

I am receiving intermittent crash reports with a stack trace that looks like this:

Caused by: java.util.ConcurrentModificationException:
   at java.util.HashMap$HashIterator.nextNode (HashMap.java:1441)
   at java.util.HashMap$KeyIterator.next (HashMap.java:1465)
   at MyPageAdapter.refreshBoundViewHolders (MyPageAdapter.java:XXX)

I don’t have any MyPageAdapter.java file in my project. There is a MyPageAdapter.kt file with XXX - 5 lines of Kotlin code in it, i.e. the stack trace points past the end of the file by 5 lines.

I read this and this so I understand that incorrect line numbers are expected when we are dealing with inline code. I am not sure if the .java in the stack trace for a Kotlin file is expected or significant. I am not particularly concerned about either of these discrepancies as I am pretty sure I can pinpoint where the crash is happening.

This is what the relevant code approximately looks like:

class MyPageAdapter(): RecyclerView.Adapter<MyViewHolder>() {
    private val boundViewHolders = HashSet<MyViewHolder>()

    override fun onBindViewHolder(holder: MyViewHolder, position: Int) {
        synchronized(boundViewHolders) {
            boundViewHolders.add(holder)
        }
    }

    override fun onViewRecycled(holder: MyViewHolder?) {
        synchronized(boundViewHolders) {
            boundViewHolders.remove(holder)
        }
        super.onViewRecycled(holder)
    }

    fun refreshBoundViewHolders() {
        synchronized(boundViewHolders) {
            boundViewHolders.forEach {
                it.refresh()
            }
        }
    }
}

The stack trace references the refreshBoundViewHolders function, so I assume that is where the exception happens even though the line number doesn’t match. I am including the other parts to show that the boundViewHolders HashSet is protected by synchronized every time it’s accessed. These are literally all the appearances of boundViewHolders in the code. Am I wrong to think that this protection should be enough to avoid a ConcurrentModificationException?

In addition, I am doubtful whether these functions are even called from anything but the main thread. I do have background threads in this app so it is not entirely out of the question, but I don’t see how it can happen.

Is it possible that the construction or deconstruction of boundViewHolders conflicts with a call to refreshBoundViewHolders and that is causing the exception? If so, how can I protect against this?

I do find it interesting that the first of those threads is about a ConcurrentModificationException in a forEach loop, which is exactly what seems to be happening to me. Is it possible that there is a problem with forEach that is causing this?

Any help or ideas would be appreciated!

2

First of all, you should remove all of the synchronization. All these methods should only be invoked on the UI/Main thread so synchronization this way just gives the illusion of doing anything (except slowing things down). I suppose that the MyViewHolder.refresh() function is a function you defined. I strongly suspect that it indirectly calls onBindViewHolder, and thus not only causes an incorrect list, it also creates a modification while you’re iterating (under water .foreach uses iteration) over the list of viewholders.

1 Like
3

That is a great observation, thanks for the insight!