Skip to content
This repository has been archived by the owner on Jan 14, 2018. It is now read-only.

ConcurrentModificationException in RequestProcessor #91

Closed
ffgiraldez opened this issue May 3, 2013 · 58 comments
Closed

ConcurrentModificationException in RequestProcessor #91

ffgiraldez opened this issue May 3, 2013 · 58 comments

Comments

@ffgiraldez
Copy link

using a SpiceService with 4 theads some times i get a ConcurrentModificationException
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:796)
at java.util.HashMap$KeyIterator.next(HashMap.java:823)
at com.octo.android.robospice.request.RequestProcessor$ResultRunnable.run(RequestProcessor.java:451)
at android.os.Handler.handleCallback(Handler.java:587)
at android.os.Handler.dispatchMessage(Handler.java:92)
at android.os.Looper.loop(Looper.java:130)
at android.app.ActivityThread.main(ActivityThread.java:3683)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:507)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:839)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:597)
at dalvik.system.NativeStart.main(Native Method)

Can robospice use ConcurrentHashMap?

@stephanenicolas
Copy link
Owner

When does the exception raise ?

@stephanenicolas
Copy link
Owner

We are open to any enhancement. Your proposal could be interesting but is vague. We can't reproduce that bug until you tell more about conditions to reproduce it.

@StingerAJ
Copy link

I also get this issue if I modify the method SpiceArrayAdapter.ThumbnailAsynTask.doInBackground like so and have 6 threads in the service:

  • out-comment if (!tempThumbnailImageFile.exists()) { and its closing brace
  • set the number of threads for the BitmapSpiceService to 6

So, I guess maybe you could reproduce it with the robospice-sample-ui-spicelist. I would have tried, but my modifications of robospice-ui-spicelist extension maybe have a sideeffect, so it would be a hassle for me to try.
This exception also doesn't always come, even with this modifications, but I just had this crash on app-startup showing a spicelist with this modified SpiceArrayAdapter and the LruCacheBitmapObjectPersister/InFileBitmapObjectPersister combo from David Stemmer.
I think the more threads are loading pictures in a big listview, the more likely you will get this exception.

I have a patchfile with a proposed modification of RequestProcessor to use ConcurrentHashMap and my own version of ConcurrentHashSet (as listRequestListenerForThisRequest needs to be a set apparently and there is no concurrent version of HashSet in the Collections-framework), how can I send the patchfile to you for testing without going through the hassle ov having to setup a branch for a pull request?

@stephanenicolas
Copy link
Owner

I need time to think about this issue. Thanx for sharing your ideas and
experience on this.

2013/5/15 StingerAJ [email protected]

I also get this issue if I modify the method
SpiceArrayAdapter.ThumbnailAsynTask.doInBackground like so and have 6
threads in the service:

  • out-comment if (!tempThumbnailImageFile.exists()) { and its closing
    brace
  • set the number of threads for the BitmapSpiceService to 6

So, I guess maybe you could reproduce it with the
robospice-sample-ui-spicelist. I would have tried, but my modifications of
robospice-ui-spicelist extension maybe have a sideeffect, so it would be a
hassle for me to try.
This exception also doesn't always come, even with this modifications, but
I just had this crash on app-startup showing a spicelist with this modified
SpiceArrayAdapter and the
LruCacheBitmapObjectPersister/InFileBitmapObjectPersister combo from David
Stemmer.
I think the more threads are loading pictures in a big listview, the more
likely you will get this exception.

I have a patchfile with a proposed modification of RequestProcessor to use
ConcurrentHashMap and my own version of ConcurrentHashSet (as
listRequestListenerForThisRequest needs to be a set apparently and there is
no concurrent version of HashSet in the Collections-framework), how can I
send the patchfile to you for testing without going through the hassle ov
having to setup a branch for a pull request?


Reply to this email directly or view it on GitHubhttps://github.com//issues/91#issuecomment-17941918
.

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - blog.octo.com
www.usievents.com
...........................................................

@prildy
Copy link

prildy commented Jun 18, 2013

I also got the same issue
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:792)
at java.util.HashMap$KeyIterator.next(HashMap.java:819)
at com.octo.android.robospice.request.RequestProcessor$ProgressRunnable.run(RequestProcessor.java:418)
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:5041)
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:793)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:560)
at dalvik.system.NativeStart.main(Native Method)

I do the network call 5 times with the timer delay per 6 sec. This exception doesn't constantly raise, but it happens at most time. Can you take a look at this issue?

@stephanenicolas
Copy link
Owner

Hi all,

Stinger, sorry for that late answer, but I would be pleased to get your
patch.

Can you create a gist maybe ?
That's a one click thing : https://gist.github.com/

Stéphane

2013/6/18 Prildy [email protected]

I also got the same issue.


Reply to this email directly or view it on GitHubhttps://github.com//issues/91#issuecomment-19586575
.

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - blog.octo.com
www.usievents.com
...........................................................

@stephanenicolas
Copy link
Owner

Oh, and I think there a git button directly close to files in the new git
interface. For a single file, that could be handy.
It's "a pull request in one click" as far a I understood.

Stéphane

2013/6/22 Stéphane Nicolas [email protected]

Hi all,

Stinger, sorry for that late answer, but I would be pleased to get your
patch.

Can you create a gist maybe ?
That's a one click thing : https://gist.github.com/

Stéphane

2013/6/18 Prildy [email protected]

I also got the same issue.


Reply to this email directly or view it on GitHubhttps://github.com//issues/91#issuecomment-19586575
.

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - blog.octo.com
www.usievents.com
...........................................................

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - blog.octo.com
www.usievents.com
...........................................................

@AndyFrench
Copy link

@stephanenicolas I seem to be getting this now using 1.4.8.

HashMap.java:792 • java.util.HashMap$HashIterator.nextEntry HashMap.java:819 • java.util.HashMap$KeyIterator.next DefaultRequestListenerNotifier.java:168 • com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run Handler.java:725 • android.os.Handler.handleCallback Handler.java:92 • android.os.Handler.dispatchMessage Looper.java:137 • android.os.Looper.loop ActivityThread.java:5227 • android.app.ActivityThread.main Method.java:-2 • java.lang.reflect.Method.invokeNative Method.java:511 • java.lang.reflect.Method.invoke ZygoteInit.java:795 • com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run ZygoteInit.java:562 • com.android.internal.os.ZygoteInit.main NativeStart.java:-2 • dalvik.system.NativeStart.main

Is there any fix?

@stephanenicolas
Copy link
Owner

We will fix this. It's really a priority issue. Nevertheless, I don't have
much time to check it right now. However, I would be very glad to accept a
pull request solving the problem (and that would pass all tests of RS).

Stéphane

2013/10/24 AndyFrench [email protected]

@stephanenicolas https://github.com/stephanenicolas I seem to be
getting this now using 1.4.8.

HashMap.java:792 • java.util.HashMap$HashIterator.nextEntry
HashMap.java:819 • java.util.HashMap$KeyIterator.next
DefaultRequestListenerNotifier.java:168 •
com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run
Handler.java:725 • android.os.Handler.handleCallback
Handler.java:92 • android.os.Handler.dispatchMessage
Looper.java:137 • android.os.Looper.loop
ActivityThread.java:5227 • android.app.ActivityThread.main
Method.java:-2 • java.lang.reflect.Method.invokeNative
Method.java:511 • java.lang.reflect.Method.invoke
ZygoteInit.java:795 •
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run
ZygoteInit.java:562 • com.android.internal.os.ZygoteInit.main
NativeStart.java:-2 • dalvik.system.NativeStart.main

Is there any fix?


Reply to this email directly or view it on GitHubhttps://github.com//issues/91#issuecomment-26968233
.

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - mobilite.octo.com
blog.octo.com - www.usievents.com
...........................................................

@siyamed
Copy link

siyamed commented Nov 11, 2013

Hi, I am using 1.4.9 and getting the same exception at DefaultRequestListenerNotifier.java line 168.

@deisold
Copy link

deisold commented Nov 25, 2013

Hello,

I'm also using 1.4.9 and getting:

java.util.ConcurrentModificationException
at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
at com.octo.android.robospice.request.notifier.SpiceServiceListenerNotifier$RequestAddedNotifier.run(SpiceServiceListenerNotifier.java:180)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:137)

If I look into the source code I see an iterating:

        for (SpiceServiceListener listener : spiceServiceListenerList) {
            listener.onRequestAdded(request, requestProcessingContext);
        }

==> The variable spiceServiceListenerList is defined as a sychronized list.
private final List spiceServiceListenerList = Collections.synchronizedList(new ArrayList());

==> If you look into the implementation of a sychronized list all public methods are synchronized against a mutux. But the iterator itself is NOT synchronized.

==> So I think that any access of such a list using an iterator has to be synchronized as well because while iterating over a synchronized list the list could be motified from any other thread because the access from outside is not globally synchronized.

Hope you find an appropriate solution!

Cheers, Dirk

@softwaremaverick
Copy link
Collaborator

I haven't looked at the code in quite a long time but if the spice service listeners are modified at runtime then yes. However,I'm sure originally they were only to be added once within the constructor of the Spice service as the listeners were designed originally to last the full life span of the object. Those listeners shouldn't necessarily be modified as part of an Activity Life cycle.

I don't know if that is still true or not.

Andrew

deisold [email protected] wrote:

Hello,

I'm also using 1.4.9 and getting:

java.util.ConcurrentModificationException
at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
at
com.octo.android.robospice.request.notifier.SpiceServiceListenerNotifier$RequestAddedNotifier.run(SpiceServiceListenerNotifier.java:180)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:137)

If I look into the source code I see an iterating:

  for (SpiceServiceListener listener : spiceServiceListenerList) {
       listener.onRequestAdded(request, requestProcessingContext);
       }

==> The variable spiceServiceListenerList is defined as a sychronized
list.
private final List spiceServiceListenerList =
Collections.synchronizedList(new ArrayList());

==> If you look into the implementation of a sychronized list all
public methods are synchronized against a mutux. But the iterator
itself is NOT synchronized.

==> So I think that any access of such a list using an iterator has to
be synchronized as well because while iterating over a synchronized
list the list could be motified from any other thread because the
access from outside is not globally synchronized.

Hope you find an appropriate solution!

Cheers, Dirk


Reply to this email directly or view it on GitHub:
#91 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@softwaremaverick
Copy link
Collaborator

The previous examples were from the Default notifier which again I don't understand as it has (last time of looking) a synchronise around it which would be the solution if not there already

Andrew Clark [email protected] wrote:

I haven't looked at the code in quite a long time but if the spice
service listeners are modified at runtime then yes. However,I'm sure
originally they were only to be added once within the constructor of
the Spice service as the listeners were designed originally to last the
full life span of the object. Those listeners shouldn't necessarily be
modified as part of an Activity Life cycle.

I don't know if that is still true or not.

Andrew

deisold [email protected] wrote:

Hello,

I'm also using 1.4.9 and getting:

java.util.ConcurrentModificationException
at
java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
at
com.octo.android.robospice.request.notifier.SpiceServiceListenerNotifier$RequestAddedNotifier.run(SpiceServiceListenerNotifier.java:180)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:137)

If I look into the source code I see an iterating:

  for (SpiceServiceListener listener : spiceServiceListenerList)

{
listener.onRequestAdded(request,
requestProcessingContext);
}

==> The variable spiceServiceListenerList is defined as a sychronized
list.
private final List spiceServiceListenerList =
Collections.synchronizedList(new ArrayList());

==> If you look into the implementation of a sychronized list all
public methods are synchronized against a mutux. But the iterator
itself is NOT synchronized.

==> So I think that any access of such a list using an iterator has to
be synchronized as well because while iterating over a synchronized
list the list could be motified from any other thread because the
access from outside is not globally synchronized.

Hope you find an appropriate solution!

Cheers, Dirk


Reply to this email directly or view it on GitHub:
#91 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@deisold
Copy link

deisold commented Nov 26, 2013

Hi, thanks a lot for your quick reply. Obviously the DefaultRequestListenerNotifier uses a synchronized block around the itereration of the list and the SpiceServiceListenerNotifier doesn't. Do you if the robospice team could provide a fix for that soon? We need to go productive within 2 weeks....

@softwaremaverick
Copy link
Collaborator

But do you actually add and remove spice s Service listeners at runtime? If so then for what purpose?

deisold [email protected] wrote:

Hi, thanks a lot for your quick reply. Obviously the
DefaultRequestListenerNotifier uses a synchronized block around the
itereration of the list and the SpiceServiceListenerNotifier doesn't.
Do you if the robospice team could provide a fix for that soon? We need
to go productive within 2 weeks....


Reply to this email directly or view it on GitHub:
#91 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@deisold
Copy link

deisold commented Nov 27, 2013

What do you mean by at runtime? We have some activities and each of them instantiates a SpiceManager which itself instantiates a SpiceServiceConnection which adds some spice service listeners in onServiceConnected at runtime.
So the service listeners are added at runtime, they are not added in a static context... Does this answer your question?

@softwaremaverick
Copy link
Collaborator

Yes thanks. I'll have to look sometime at how robospice now works with them. I'm sure the last time I saw them you couldn't remove them. I'll have to check it out. However, this can only be done at evenings and weekends as I work on other stuff during the day.

deisold [email protected] wrote:

What do you mean by at runtime? We have some activities and each of
them instantiates a SpiceManager which itself instantiates a
SpiceServiceConnection which adds some spice service listeners in
onServiceConnected at runtime.
So the service listeners are added at runtime, they are not added in a
static context... Does this answer your question?


Reply to this email directly or view it on GitHub:
#91 (comment)

Sent from my Android phone with K-9 Mail. Please excuse my brevity.

@softwaremaverick
Copy link
Collaborator

@stephanenicolas if you are releasing soon for the next RS I believe there could be a simple change in SpiceServiceListenerNotifier for this the last time I looked. By adding as specified above the synchronized around the list iterator. I'll look again to confirm that this is modifiable multiple times but it would be good to add this simple change anyway for release.

@stephanenicolas
Copy link
Owner

Sorry for the long delay to answer that thread.

I just added @softwaremaverick 's changes to RS. All tests seem to pass locally (even if Travis still has some pain on some tests). I am deploying a new snapshot right now. Does one of you wanna try it ?

I could release today if receive a go from one of you.

Thanks @softwaremaverick, @deisold , @siyamed and @ffgiraldez for finding and patching the bug.

@sayah-y
Copy link

sayah-y commented Jul 1, 2014

Hello,

I'm using 1.4.12. I do the network call many times and I'm getting:

E/AndroidRuntime(4234): java.util.ConcurrentModificationException
E/AndroidRuntime(4234): at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
E/AndroidRuntime(4234): at java.util.HashMap$KeyIterator.next(HashMap.java:833)
E/AndroidRuntime(4234): at com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)

Thanks

@stephanenicolas
Copy link
Owner

Can you try our 1.4.13 snapshots and see problem persists ?

Thx,
S.

2014-07-01 15:54 GMT+02:00 sayah-y [email protected]:

Hello,

I'm using 1.4.12. I do the network call many times and I'm getting:

E/AndroidRuntime(4234): java.util.ConcurrentModificationException
E/AndroidRuntime(4234): at
java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
E/AndroidRuntime(4234): at
java.util.HashMap$KeyIterator.next(HashMap.java:833)
E/AndroidRuntime(4234): at
com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)

Thanks


Reply to this email directly or view it on GitHub
#91 (comment)
.

@escarti
Copy link

escarti commented Jul 7, 2014

I'm having the same issue as sayah-y. 1.4.13 is not on maven.

Will it be available there at some point?

Thnx.

@stephanenicolas
Copy link
Owner

Hi escarti,

1.4.13 is planned to be released soon. We just have one feature branch to
review before leaving.

Other contributors are working on it.

Look at the RS wiki, you will find how to use the 1.4.13 snapshots.

Stephane
Le 7 juil. 2014 13:43, "escarti" [email protected] a écrit :

I'm having the same issue as sayah-y. 1.4.13 is not on maven.

Will it be available there at some point?

Thnx.


Reply to this email directly or view it on GitHub
#91 (comment)
.

@escarti
Copy link

escarti commented Jul 7, 2014

Hi Stephane,

thanks for the info, I managed to use the 1.4.13-SNAPSHOT but there are serveral things:

  1. On the wiki for using snapshots you write:
    Add this configuration to your build.gradle file, in the dependencies ( <- I think this should be repositories ) block:
    maven { url "https://oss.sonatype.org/content/repositories/snapshots/" }
  2. The error still occurs:
    Process: net.appbounty.android, PID: 27894
    java.util.ConcurrentModificationException
    at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
    at java.util.HashMap$KeyIterator.next(HashMap.java:833)
    at com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)

Looking at the code

synchronized (listeners) {
for (final RequestListener<?> listener : listeners) {
if (listener != null) {

My guess is that you are synchronizing the Set<RequestListener> listeners but not the RequestListener inside it. I think adding

synchronized (listeners) {
for (final RequestListener<?> listener : listeners) {
synchrnoized(litener) {
if (listener != null) {

will solve the problem, but I might be wrong and I didn't have the chance to test it.

Hope to be of some help.

Thanks for the amazing work!

Cheers,
Edu.

@stephanenicolas
Copy link
Owner

You also need to upgrade to 1.4.13-SNASPHOT version of RS ! :)

2014-07-07 17:49 GMT+02:00 escarti [email protected]:

Hi Stephane,

thanks for the info, I managed to use the 1.4.13-SNAPSHOT but there are
serveral things:

On the wiki for using snapshots you write:
Add this configuration to your build.gradle file, in the dependencies
( <- I think this should be repositories ) block:
maven { url "https://oss.sonatype.org/content/repositories/snapshots/"
}
2.

The error still occurs:
Process: net.appbounty.android, PID: 27894
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
at java.util.HashMap$KeyIterator.next(HashMap.java:833)
at
com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)

Looking at the code

synchronized (listeners) {
for (final RequestListener<?> listener : listeners) {
if (listener != null) {

My guess is that you are synchronizing the Set> listeners but not the
RequestListener<?> inside it. I think adding

synchronized (listeners) {
for (final RequestListener<?> listener : listeners) {
synchrnoized(litener) {
if (listener != null) {

will solve the problem, but I might be wrong and I didn't have the chance
to test it.

Hope to be of some help.

Thanks for the amazing work!

Cheers,
Edu.


Reply to this email directly or view it on GitHub
#91 (comment)
.

@stephanenicolas
Copy link
Owner

I mean, not only to add the snapshot repo.

2014-07-07 18:19 GMT+02:00 Stéphane NICOLAS [email protected]:

You also need to upgrade to 1.4.13-SNASPHOT version of RS ! :)

2014-07-07 17:49 GMT+02:00 escarti [email protected]:

Hi Stephane,

thanks for the info, I managed to use the 1.4.13-SNAPSHOT but there are
serveral things:

On the wiki for using snapshots you write:
Add this configuration to your build.gradle file, in the dependencies
( <- I think this should be repositories ) block:
maven { url "https://oss.sonatype.org/content/repositories/snapshots/"
}
2.

The error still occurs:
Process: net.appbounty.android, PID: 27894
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
at java.util.HashMap$KeyIterator.next(HashMap.java:833)
at
com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)

Looking at the code

synchronized (listeners) {
for (final RequestListener<?> listener : listeners) {
if (listener != null) {

My guess is that you are synchronizing the Set> listeners but not the
RequestListener<?> inside it. I think adding

synchronized (listeners) {
for (final RequestListener<?> listener : listeners) {
synchrnoized(litener) {
if (listener != null) {

will solve the problem, but I might be wrong and I didn't have the chance
to test it.

Hope to be of some help.

Thanks for the amazing work!

Cheers,
Edu.


Reply to this email directly or view it on GitHub
#91 (comment)
.

@escarti
Copy link

escarti commented Jul 7, 2014

Of course I upgraded.

I used:

compile 'com.octo.android.robospice:robospice:1.4.13-SNAPSHOT'
compile 'com.octo.android.robospice:robospice-spring-android:1.4.12'

Cheers,
Edu.

@escarti
Copy link

escarti commented Jul 8, 2014

Hi Stéphane,

I did a project clean and a gradle sync so I assume that the 1.4.13-snapshot is the new one.

With an exception breakpoint I saw that the listeners collection has just one item and that item is created in here:

spiceManager.execute(postOfferClick, "offer_click", DurationInMillis.ALWAYS_EXPIRED, new PostOfferClickRequestListener());

The full stack trace is:

07-08 12:46:12.448 22861-22861/net.appbounty.android E/AndroidRuntime﹕ FATAL EXCEPTION: main
Process: net.appbounty.android, PID: 22861
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
at java.util.HashMap$KeyIterator.next(HashMap.java:833)
at com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:136)
at android.app.ActivityThread.main(ActivityThread.java:5001)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:515)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
at dalvik.system.NativeStart.main(Native Method)

@stephanenicolas
Copy link
Owner

Could you check the class of the set of listeners inside the runnable ?

I wonder if it is truly a synchronized set and if so, the bug is somewhere
else or if it's not a synchronized set and then the problem is that it
should be : somewhere in the code we use a wring set.

Thx for all your efforts, we will a special 1.4.13 for you when it works
and snapshots so that you can test our patches.

Stéphane
Le 8 juil. 2014 12:51, "escarti" [email protected] a écrit :

Hi Stéphane,

I did a project clean and a gradle sync so I assume that the
1.4.13-snapshot is the new one.

With an exception breakpoint I saw that the listeners collection has just
one item and that item is created in here:

spiceManager.execute(postOfferClick, "offer_click",
DurationInMillis.ALWAYS_EXPIRED, new PostOfferClickRequestListener());

The full stack trace is:

07-08 12:46:12.448 22861-22861/net.appbounty.android E/AndroidRuntime﹕
FATAL EXCEPTION: main
Process: net.appbounty.android, PID: 22861
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
at java.util.HashMap$KeyIterator.next(HashMap.java:833)
at
com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:136)
at android.app.ActivityThread.main(ActivityThread.java:5001)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:515)
at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
at dalvik.system.NativeStart.main(Native Method)


Reply to this email directly or view it on GitHub
#91 (comment)
.

@stephanenicolas
Copy link
Owner

BTW,

If you could find which OTHER thread is modifying the listener set while
this thread is iterating over it, that would tremendously help. You IDE
should provide this data to you...

S.
Le 8 juil. 2014 12:55, "Stéphane NICOLAS" [email protected] a écrit
:

Could you check the class of the set of listeners inside the runnable ?

I wonder if it is truly a synchronized set and if so, the bug is somewhere
else or if it's not a synchronized set and then the problem is that it
should be : somewhere in the code we use a wring set.

Thx for all your efforts, we will a special 1.4.13 for you when it works
and snapshots so that you can test our patches.

Stéphane
Le 8 juil. 2014 12:51, "escarti" [email protected] a écrit :

Hi Stéphane,

I did a project clean and a gradle sync so I assume that the
1.4.13-snapshot is the new one.

With an exception breakpoint I saw that the listeners collection has just
one item and that item is created in here:

spiceManager.execute(postOfferClick, "offer_click",
DurationInMillis.ALWAYS_EXPIRED, new PostOfferClickRequestListener());

The full stack trace is:

07-08 12:46:12.448 22861-22861/net.appbounty.android E/AndroidRuntime﹕
FATAL EXCEPTION: main
Process: net.appbounty.android, PID: 22861
java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:806)
at java.util.HashMap$KeyIterator.next(HashMap.java:833)
at
com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:136)
at android.app.ActivityThread.main(ActivityThread.java:5001)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:515)
at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
at dalvik.system.NativeStart.main(Native Method)


Reply to this email directly or view it on GitHub
#91 (comment)
.

@escarti
Copy link

escarti commented Jul 8, 2014

Hi Stéphane,

after:

  • app deletion
  • project clean
  • gradle resynch
  • usage of the latest 1.4.13-snapshot from yesterday

I haven't experience the crash for quite some time. Sadly I cannot guarantee that it's not there since I cannot reproduce it, but so far so good.

I tried to reproduce the error on debug mode for 45 minutes and didn't happen.

Thanks a lot for your support and if I experience it again I will send you the thread causing the issue and as much info as I possibly can.

Cheers,
Edu.

@stephanenicolas
Copy link
Owner

Ahhhhh :)

I was really wondering what else could cause the bug. Nice !

Thx for your continuous feedback and testing.

Stéphane

2014-07-08 16:10 GMT+02:00 escarti [email protected]:

Hi Stéphane,

after:

  • app deletion
  • project clean
  • gradle resynch
  • usage of the latest 1.4.13-snapshot from yesterday

I haven't experience the crash for quite some time. Sadly I cannot
guarantee that it's not there since I cannot reproduce it, but so far so
good.

I tried to reproduce the error on debug mode for 45 minutes and didn't
happen.

Thanks a lot for your support and if I experience it again I will send you
the thread causing the issue and as much info as I possibly can.

Cheers,
Edu.


Reply to this email directly or view it on GitHub
#91 (comment)
.

@stephanenicolas
Copy link
Owner

So this bug is solved in 1.4.13 and its snapshot.

stephanenicolas added a commit that referenced this issue Jul 8, 2014
stephanenicolas added a commit that referenced this issue Jul 8, 2014
@escarti
Copy link

escarti commented Jul 8, 2014

Yes, my guess is that it needed a clean and after that it used the second version of the snapshot.

So to sum up: Your latest 1.4.13-snapshot seems to be the cure :)

Thanx! This bug was causing me a lot of headaches!

@escarti
Copy link

escarti commented Jul 9, 2014

Hi Stéphane,

I'm sorry to bring bad news again, but the bug re-appeared. I gathered some new info that might help.

The error apparently appears when I do 2 times the same API call from different locations.

I call my API call from a background service and from a fragment.

The exact point where the exception is thrown is

HashMapEntry<K, V> nextEntry() {
if (modCount != expectedModCount)
throw new ConcurrentModificationException();

the expectedModCount is 2 whereas the modCount is 3

I will send you the full Thread export and a screenshot of the variables.

Hopefully it will help.

Cheers,
Edu.

@stephanenicolas
Copy link
Owner

Nuts ! ;)

I look at the screenshot.

2014-07-09 10:59 GMT+02:00 escarti [email protected]:

Hi Stéphane,

I'm sorry to bring bad news again, but the bug re-appeared. I gathered
some new info that might help.

The error apparently appears when I do 2 times the same API call from
different locations.

I call my API call from a background service and from a fragment.

The exact point where the exception is thrown is

HashMapEntry nextEntry() {
if (modCount != expectedModCount)
throw new ConcurrentModificationException();

the expectedModCount is 2 whereas the modCount is 3

I will send you the full Thread export and a screenshot of the variables.

Hopefully it will help.

Cheers,
Edu.


Reply to this email directly or view it on GitHub
#91 (comment)
.

@escarti
Copy link

escarti commented Jul 9, 2014

I implemented a workaround and now I have a "lock" and manually synchronize the api calls to ensure that only one either the service or the fragment are using robospice.

So far so good.

If you need more info let me know.

@stephanenicolas
Copy link
Owner

Yes I do. I really don't get it.

2014-07-09 15:53 GMT+02:00 escarti [email protected]:

I implemented a workaround and now I have a "lock" and manually
synchronize the api calls to ensure that only one either the service or the
fragment are using robospice.

So far so good.

If you need more info let me know.


Reply to this email directly or view it on GitHub
#91 (comment)
.

@escarti
Copy link

escarti commented Jul 9, 2014

Since I now know what is causing it I managed to reproduce it pretty consistently.

I got the service performing a GET Api call every second and I can force the fragment to do an api call on click. In this way I manage to get the crash pretty consistently.

How should we proceed? Skype and screen share?

@stephanenicolas
Copy link
Owner

On which time zone are you ?

Really, I can't find anything that could explain this bug..

2014-07-09 16:38 GMT+02:00 escarti [email protected]:

Since I now know what is causing it I managed to reproduce it pretty
consistently.

I got the service performing a GET Api call every second and I can force
the fragment to do an api call on click. In this way I manage to get the
crash pretty consistently.

How should we proceed? Skype and screen share?


Reply to this email directly or view it on GitHub
#91 (comment)
.

@stephanenicolas
Copy link
Owner

Eduardo and I could finally find the cause of the problem.

If you get this exception, it is highly probable that you are either :

  • cancelling a request
  • stopping the SpiceManager

from within a request listener's callback method. And you simply can't do this. That's not an RS design problem but more a limitation of java language itself : while iterating through the synchronized set of listeners associated to a request, this set of listeners can't be modified.

A workaround could have been to use an CopyOnWriteArray but it comes with performance penalty. Moreover, there is no reason to stop the SpiceManager after a request is complete: RS does it automatically for you, it will shutdown once there are no requests to execute and restart if a new request is executed.

Note that a FAQ entry has been added on this matter.

@softwaremaverick
Copy link
Collaborator

I noticed that in the SpiceManager the set of listeners is not always syncrhonised. Not sure if that's the cause of the spice manager issue you're seeing Stephan.

There are various places where there's a Map of request/listeners, eg.

    private final Map<CachedSpiceRequest<?>, Set<RequestListener<?>>> mapRequestToLaunchToRequestListener = 

Collections
        .synchronizedMap(new IdentityHashMap<CachedSpiceRequest<?>, Set<RequestListener<?>>>());

In various locations it checks whether the list of listeners is null or not, if true it creates it.

This happens in Request Processor and Spice Manager. Typically it creates it with a synchronized set, eg.

Request Processor addRequest

        if (listRequestListenerForThisRequest == null) {
            if (request.isProcessable()) {
                Ln.d("Adding entry for type %s and cacheKey %s.", request.getResultType(), 

request.getRequestCacheKey());
                listRequestListenerForThisRequest = Collections.synchronizedSet(new HashSet<RequestListener<?

>>());
                this.mapRequestToRequestListener.put(request, listRequestListenerForThisRequest);
            }
        } else {

Spice Manager onRequestAggregated

            Set<RequestListener<?>> listeners = mapPendingRequestToRequestListener.get(cachedSpiceRequest);
            if (listeners == null) {
                listeners = Collections.synchronizedSet(new HashSet<RequestListener<?>>());
                mapPendingRequestToRequestListener.put(cachedSpiceRequest, listeners);
            }

Whilst SpiceManager addRequest doesn't....

    private <T> void addRequestListenerToListOfRequestListeners(final CachedSpiceRequest<T> 

cachedSpiceRequest, final RequestListener<T> requestListener) {
        synchronized (mapRequestToLaunchToRequestListener) {
            Set<RequestListener<?>> listeners = mapRequestToLaunchToRequestListener.get(cachedSpiceRequest);
            if (listeners == null) {
                listeners = new HashSet<RequestListener<?>>();
                this.mapRequestToLaunchToRequestListener.put(cachedSpiceRequest, listeners);
            }
            listeners.add(requestListener);
        }
    }

Which is called from

    public <T> void execute(final CachedSpiceRequest<T> cachedSpiceRequest, final RequestListener<T> 

requestListener) {
        addRequestListenerToListOfRequestListeners(cachedSpiceRequest, requestListener);
        Ln.d("adding request to request queue");
        this.requestQueue.add(cachedSpiceRequest);
    }

The SpiceManager seems to send this set of listeners to the service in

    private void sendRequestToService(final CachedSpiceRequest<?> spiceRequest) {
        lockSendRequestsToService.lock();
        try {
            if (spiceRequest != null && spiceService != null) {
                if (isStopped) {
                    spiceService.addRequest(spiceRequest, null);
                } else {
                    final Set<RequestListener<?>> listRequestListener = 

mapRequestToLaunchToRequestListener.get(spiceRequest);
                    Ln.d("Sending request to service : " + spiceRequest.getClass().getSimpleName());
                    spiceService.addRequest(spiceRequest, listRequestListener);
                }
            }
        } finally {
            lockSendRequestsToService.unlock();
        }
    }

Although what I'm seeing in the Spice Service at that time is that it simply does an addAll on the listeners. Unless a different implementation does something else.

@stephanenicolas
Copy link
Owner

Thx @softwaremaverick , but that is exactly what my last commit changes ;)
(addRequest)

And you're perfectly right, that caused the bug. This plus the explanation given in my last comment.
Btw, you really strong in java concurrency ;)

@nugmanovagulnaz
Copy link

I have BroadcastReceiver. I'm calling start of spiceManager in onReceive method:
public void onReceive(Context context, Intent intent) {
spiceManager.start(context);
...
}
When should i call the shouldStop method? There are no onStop() method in BroadcastReceiver.

@softwaremaverick
Copy link
Collaborator

Yeah you should do very little in a broadcast receiver. I recommend sending the intent to a service to do further processing

On 20 August 2014 13:06:41 BST, nugmanovagulnaz [email protected] wrote:

I have BroadcastReceiver. I'm calling start of spiceManager in
onReceive method:
public void onReceive(Context context, Intent intent) {
spiceManager.start(context);
...
}
When should i call the shouldStop method? There are no onStop() method
in BroadcastReceiver.


Reply to this email directly or view it on GitHub:
#91 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@stephanenicolas
Copy link
Owner

I guess that from a broadcast receiver you don't expect to do something in
the callback (listener) of the request.
If you want to do so, follow @softwaremaverick advice then and delegate to
another entity with a lifecycle.
If not, they fire your request and stop the manager afterwards, your
request will be executed but no call back will be triggered, you can for
instance fill a cache that way.

Stéphane

2014-08-20 13:13 GMT-04:00 softwaremaverick [email protected]:

Yeah you should do very little in a broadcast receiver. I recommend
sending the intent to a service to do further processing

On 20 August 2014 13:06:41 BST, nugmanovagulnaz [email protected]
wrote:

I have BroadcastReceiver. I'm calling start of spiceManager in
onReceive method:
public void onReceive(Context context, Intent intent) {
spiceManager.start(context);
...
}
When should i call the shouldStop method? There are no onStop() method
in BroadcastReceiver.


Reply to this email directly or view it on GitHub:

#91 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.


Reply to this email directly or view it on GitHub
#91 (comment)
.

@nugmanovagulnaz
Copy link

Well, i created IntentService that handles my broadcastReceivers result.
But there is no onStop() in IntentService too. Where should i call spiceManager.shouldStop() in IntentService?
I'm sorry for bothering you

@stephanenicolas
Copy link
Owner

IntentService is almost the same as broadcast receiver, a simple life
cycle.
What do you want to do when you get your data from the network ?

Stéphane

2014-08-21 7:38 GMT-04:00 nugmanovagulnaz [email protected]:

Well, i created IntentService that handles my broadcastReceivers result.
But there is no onStop() in IntentService too. Where should i call
spiceManager.shouldStop() in IntentService?
I'm sorry for bothering you


Reply to this email directly or view it on GitHub
#91 (comment)
.

@nugmanovagulnaz
Copy link

When i get data from network i want to send another broadcast with this data and it's all.

@softwaremaverick
Copy link
Collaborator

Sounds like you need a full service to me!

Or! And this is untested but potentially going to work, you could create a spice service with a custom (and I can't remember what I called it) oh... Notifier?

As in you could create a custom result notifier which sent the result as a broadcast. The notifiers run in a non ui thread so you don't need to worry about them affecting ui.

On 21 August 2014 19:14:44 BST, nugmanovagulnaz [email protected] wrote:

When i get data from network i want to send another broadcast with this
data and it's all.


Reply to this email directly or view it on GitHub:
#91 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@nugmanovagulnaz
Copy link

What do you mean under a full service? android.app.Service? Service hasn't got onStop() too

@Alansc
Copy link

Alansc commented Sep 12, 2014

Hi All,

Any news about the issue #91? I'm using the version 1.4.13 and it still has the problem:

E/AndroidRuntime(9608): FATAL EXCEPTION: main
E/AndroidRuntime(9608): java.util.ConcurrentModificationException
E/AndroidRuntime(9608): at java.util.HashMap$HashIterator.nextEntry(HashMap.java:792)
E/AndroidRuntime(9608): at java.util.HashMap$KeyIterator.next(HashMap.java:819)
E/AndroidRuntime(9608): at com.octo.android.robospice.request.notifier.DefaultRequestListenerNotifier$ResultRunnable.run(DefaultRequestListenerNotifier.java:168)
E/AndroidRuntime(9608): at android.os.Handler.handleCallback(Handler.java:725)
E/AndroidRuntime(9608): at android.os.Handler.dispatchMessage(Handler.java:92)
E/AndroidRuntime(9608): at android.os.Looper.loop(Looper.java:137)

Could you someone help me? Any patch to fix it?

Many thanks,
Alan

@stephanenicolas
Copy link
Owner

Please, see FAQ entry about this exception. Chances are it comes from your code (trying to modify the list of listeners during the call back. For instance by stopping the spice manager).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests