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

AWSIotMqttManager messagequeue #532

Merged
merged 2 commits into from
Oct 11, 2018
Merged

AWSIotMqttManager messagequeue #532

merged 2 commits into from
Oct 11, 2018

Conversation

rvp-thunderbuild
Copy link
Contributor

@rvp-thunderbuild rvp-thunderbuild commented Sep 20, 2018

Occasionally the AWSIotMqttManager throws the following exception on the main thread:
java.lang.NullPointerException: Attempt to read from field 'java.lang.Object java.util.LinkedList$Node.item' on a null object reference at java.util.LinkedList.unlink(LinkedList.java:211) at java.util.LinkedList.remove(LinkedList.java:526) at com.amazonaws.mobileconnectors.iot.AWSIotMqttManager.publishMessagesFromQueue(AWSIotMqttManager.java:1224)

Which will cause the entire app to crash.
This was reported earlier in #421 and never fixed.

I haven't been able to consistently reproduce this issue because it seems to be timing related where multiple threads are removing items from the message queue. In order to solve this problem I propose the following two fixes:

  1. Replace the LinkedList with a ConcurrentLinkedQueue in order to allow better access for multiple threads.
  2. Use a looper of a thread different from the main thread, since there is absolutely no need to push queued message from the main thread.

@desokroshan desokroshan requested a review from scb01 September 24, 2018 18:38
@frankmuellr frankmuellr added iot Issues with the AWS Android SDK for Internet of Things (IoT) pull request labels Sep 28, 2018
Copy link
Contributor

@mutablealligator mutablealligator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving the change to move from LinkedList to ConcurrentLinkedQueue. The HandlerThread->Looper change will be done separately.

@mutablealligator mutablealligator merged commit 010bb23 into aws-amplify:master Oct 11, 2018
@rvp-thunderbuild
Copy link
Contributor Author

@kvasukib When will the Looper issue be addressed? And where can I follow this issue?

@mutablealligator
Copy link
Contributor

@rvp-thunderbuild For the looper change, we need to ensure that the handler thread quits gracefully. Otherwise, there will be a lot of threads lying around without being garbage collected. You are welcome to submit a PR with the additional change of quitting the handler thread appropriately or open a new issue. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iot Issues with the AWS Android SDK for Internet of Things (IoT)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants