Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

ConcurrentModificationException in the Hue SDK #7

Open
ghost opened this issue Aug 28, 2014 · 6 comments
Open

ConcurrentModificationException in the Hue SDK #7

ghost opened this issue Aug 28, 2014 · 6 comments

Comments

@ghost
Copy link

ghost commented Aug 28, 2014

We are busy with a Hue app on Android, and often when the bridge becomes unavailable we get the error below. Our implementation of PHSDKListener will post all callbacks to a handler, so we are pretty sure it is not something in our code. Can you please look into it?

Process: <MyProcess>, PID: 10493
java.util.ConcurrentModificationException
at java.util.ArrayList$ArrayListIterator.next(ArrayList.java:573)
at com.philips.lighting.hue.sdk.notification.impl.PHNotificationManagerImpl$1.onReceived(PHNotificationManagerImpl.java:128)
at com.philips.lighting.hue.sdk.notification.impl.PHHueResultReceiver.execute(PHHueResultReceiver.java:26)
at com.philips.lighting.hue.sdk.notification.impl.PHNotificationManagerImpl.notifySDKError(PHNotificationManagerImpl.java:136)
at com.philips.lighting.hue.sdk.heartbeat.PHHeartbeatProcessor.run(PHHeartbeatProcessor.java:132)
at java.util.Timer$TimerImpl.run(Timer.java:284)
@SteveyO
Copy link
Contributor

SteveyO commented Sep 15, 2014

Firstly apologies for the delay in responding. I have an issue with github notifications but no excuse.
Are you still experiencing this problem? If so do you have a reproducible scenario?

I have checked the code and I can't see anywhere which can result in a ConcurrentModificationException. The onReceived method (PHNotification manager) is just calling the onError method in the PHSDKListener (which is created by yourself). If you are still experiencing issues, can you check your onError method (in your PHSDKListener object).
Thanks

@SteveyO SteveyO closed this as completed Oct 7, 2014
@richardmcclellan
Copy link

I am seeing this issue as well. I'd appreciate it if someone could look into it again, thanks!

@SteveyO SteveyO reopened this Nov 12, 2014
@SteveyO
Copy link
Contributor

SteveyO commented Nov 12, 2014

Sure, I will take another look. If you have any more details, as to what happened exactly to cause the exception it would be helpful.

@iagreen
Copy link

iagreen commented Dec 2, 2014

@SteveyO I have been debugging this, and I think it occurs when we call registerSDKListener() on the NotificationManager from our app on the main thread while the onError callbacks are being fired on a background thread. Are you traversing the list of listeners on a background thread?

@SteveyO
Copy link
Contributor

SteveyO commented Dec 12, 2014

@iagreen Thanks, this info was helpful, I have been looking at this issue again.
Yes, calling the registerSDKListener adds a listener to a list. And the onError is being fired on a different thread (and this traverses through this list list) so I can see how a concurrentModificationException could be thrown here.

Generally in the SDK, any http call (heartbeat, updates, find new lights etc) is called on a separate thread and the onError is called from this thread.

However, saying that am still a little baffled. Normally, I would expect the registerSDKListener to be called when the app is first created and only once (before any http calls are made and before the connection to the bridge has been established).
Could you please clarify when you are registering the listener?
i.e. Calling phHueSDK.getNotificationManager().registerSDKListener(listener);

And I will take it from there. Sorry I don't have anything more concrete, it's a tricky one this.

@iagreen
Copy link

iagreen commented Dec 12, 2014

We were accidentally registering the listener way more than we should have
been, which is how I noticed where this error came from. That has been
fixed on our end. As you say, if we register once then you will never run
into the issue, but we do actually register two listeners at different
points in our app, so in theory still open to getting this crash.

On Fri, Dec 12, 2014 at 5:05 AM, Steve notifications@github.com wrote:

@iagreen https://github.com/iagreen Thanks, this info was helpful, I
have been looking at this issue again.
Yes, calling the registerSDKListener adds a listener to a list. And the
onError is being fired on a different thread (and this traverses through
this list list) so I can see how a concurrentModificationException could be
thrown here.

Generally in the SDK, any http call (heartbeat, updates, find new lights
etc) is called on a separate thread and the onError is called from this
thread.

However, saying that am still a little baffled. Normally, I would expect
the registerSDKListener to be called when the app is first created and only
once (before any http calls are made and before the connection to the
bridge has been established).

Could you please clarify when you are registering the listener?

i.e. Calling
phHueSDK.getNotificationManager().registerSDKListener(listener);

And I will take it from there. Sorry I don't have anything more concrete,
it's a tricky one this.


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

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

3 participants