Does this actually fix the problem?

toList is still doing iteraction over a collection. If wireframes is a MutableList and during this iteration some other thread changes it, then ConcurrentModificationException can be thrown.

Yes it does, toList creates a new list instance of original one, so when you modifying the original list, the new list instance will not be affected anymore during the iteration.

This can be proved by the unit test I added, when removing toList the test fails, when adding it the test passes

creates a new list instance of original one

it does it doing an iteraction over an original list. It all comes down to this (copying ArrayList using a constructor)

if during this copying another thread modifies the original list, the copyied list will be in invalid state or copying will crash

Most probably toList doesn't create iterator, but is using System.arrayCopy under the hood, which is native, so behavior may be different in case of concurrent modification.

Indeed, using the test provided outcome is different with and without toList call.

yes, ConcurrentModificationException is thrown when the code of the collection itself is able to detect that something is wrong (concurrent modification occured).

The fact that it isn't thrown doesn't mean that the code is thread-safe.

System.arrayCopy as it is native most likely can't throw this exception. But it is doing the copy of the array without any locking. So if array is concurrently modified, the result of the copy could be bad.

If you create a MutableList, populate it with values and then never modify it again, then at some point pass it to another thread using executorService.execute, it should be fine.

So the only possibility as far as I understand is that the MutableList is modified after it is passed to another thread.

Does it work in the way that we create these wireframes on UI thread and then pass them to the worker thread and they are only read from there? If so, it's strange that we have such problem.

theoretically session replay wireframes generation works like you described, it should never be modified afterwards. But like I said we currently not able to locate the place where it is modified, only thing we can do is to prevent exception from modification.

I think the problem lies here:

and is because you are using this callback on the UIThread. So in this moment you can have a WorkerThread iterating your wireframes from one side and this callback iterating the same collection on the UIThread. None of this logic is doing any alteration though on the collection so not 100% sure how the ConcurrentModificationException might be triggered but I am pretty sure is somewhere around this logic.

Also @ambushwork even if you're doing a copy there you need to synchronise the code block where you are actually performing the copy at it might happen to have a concurrent modification in the middle of the copy operation.

As I explained to Sasha, adding synchronise block can bring performance punishment, and since the crash rate of this ConcurrentModificationException is already very low before this fix, the chance of triggering ConcurrentModificationException when copying a small size of list can be negligible, I think it's a good trade-off