Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception: java.util.ConcurrentModificationException on 1.1.1-rc1 #205

Open
megataps opened this issue Jul 4, 2018 · 5 comments
Open

Exception: java.util.ConcurrentModificationException on 1.1.1-rc1 #205

megataps opened this issue Jul 4, 2018 · 5 comments
Milestone

Comments

@megataps
Copy link

megataps commented Jul 4, 2018

I got this crash report on the latest build 1.1.1-rc1

Fatal Exception: java.util.ConcurrentModificationException
at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
at com.android.volley.toolbox.ImageLoader$4.run(SourceFile:491)
at android.os.Handler.handleCallback(Handler.java:739)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:135)
at android.app.ActivityThread.main(ActivityThread.java:5318)
at java.lang.reflect.Method.invoke(Method.java)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:922)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:717)

@hero0812
Copy link

hero0812 commented Jul 6, 2018

do not modify the ArrayList when it is being traversed

@jpd236
Copy link
Collaborator

jpd236 commented Jul 9, 2018

Assuming the line in question is indeed line 491 of ImageLoader.java:

for (ImageContainer container : bir.mContainers) {

then the issue would be something mutating mContainers during this iteration.

This line of code always runs on the main thread (it is posted to a main thread handler). There are four other methods which mutate mContainers, but as far as I can tell, all of these are only called by methods which first invoke Threads.throwIfNotOnMainThread(), and so would throw a different failure if they were being called here on a different thread.

There are thus only two possibilities I can think of:

  • Something inside this loop is causing the list to be mutated. The ImageListeners created by Volley don't appear to ever do this. Are you using a custom ImageListener, and if so, does that ImageListener ever call ImageContainer#cancelRequest or ImageLoader#get?
  • This isn't actually the line of code that's crashing. (Proguard appears to be involved, since it says "SourceFile" rather than the actual file).

I don't think this is a regression from older builds. Sounds like this was a one-off crash reported in the wild? Do you have more information (and ideally code) showing how you are using ImageLoader?

@jpd236 jpd236 added this to the 1.2.0 milestone Jul 9, 2018
@jpd236
Copy link
Collaborator

jpd236 commented May 18, 2020

https://stackoverflow.com/questions/35479128/volley-image-loader-concurrentmodificationexception seems to be a similar crash with sample code in the wild.

@javad-vovin
Copy link

javad-vovin commented Dec 8, 2020

We have a similar exception.
It's very hard to reproduce, but the scenario is ImageContainer cancel request but handler in batchResponse is still processing mBatchedResponses, when this happen java.util.ConcurrentModificationException will occurred.

@joebowbeer
Copy link
Contributor

joebowbeer commented Dec 8, 2020

CME is most often thrown from a single thread, despite the name, so that seems most likely here.

These iterators are both in flight when a listener may be called on the same thread:

for (BatchedImageRequest bir : mBatchedResponses.values()) {
    for (ImageContainer container : bir.mContainers) {

Maybe the problem can be avoided by switching to indexed loops:

for (int i = 0; i < bir.mContainers.size(); i++) {
    ImageContainer container = bir.mContainers.get(i);

Or copying to an array before iterating, depending on the desired behavior.

The outer batchedResponses loop should also be protected from concurrent modification in the same way. An invoked listener might be able to reach:

mBatchedResponses.remove(mCacheKey)

@jpd236 jpd236 modified the milestones: 1.2.0, 1.2.1 Feb 15, 2021
@jpd236 jpd236 modified the milestones: 1.2.1, 1.2.2 Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants