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

0.14: ConcurrentModificationException in MemorySubscriptionsRepository #571

Closed
hylkevds opened this issue Feb 2, 2021 · 1 comment
Closed

Comments

@hylkevds
Copy link
Collaborator

hylkevds commented Feb 2, 2021

The MemorySubscriptionsRepository implementation does not synchronise write access to the subscriptions list. As a result, Iterations over the list will break when another Thread modifies the list.

I'm not sure what the best way to solve this is:

  • the easy way is to use a CopyOnWriteArrayList to protect iterations, and to synchronise add and remove operations. This would not be good for add/remove performance.
  • Another option is to use a ConcurrentSkipListSet that has better add/remove performance, but that would mean the interface of the public List<Subscription> listAllSubscriptions() method would need to change to return a Set.

What do you think?
I'll make a PR for the change to ConcurrentSkipListSet. The other one is pretty trivial.

Expected behavior

No exceptions

Actual behavior

ConcurrentModificationExceptions in MemorySubscriptionsRepository

Steps to reproduce

Have many clients subscribe and unsubscribe often, while publishes are in progress.

Minimal yet complete reproducer code (or URL to code) or complete log file

Moquette MQTT version

0.14

JVM version (e.g. java -version)

OpenJDK 11

OS version (e.g. uname -a)

Linux

hylkevds added a commit to FraunhoferIOSB/moquette that referenced this issue Feb 2, 2021
…riptionsRepository

The MemorySubscriptionsRepository implementation does not synchronise
write access to the subscriptions list. As a result, Iterations over the
list will break when another Thread modifies the list.
This changes the ArrayList to a ConcurrentSkipListSet and changes the
interface of the public List<Subscription> listAllSubscriptions() method
to return a Set.
hylkevds added a commit to FraunhoferIOSB/moquette that referenced this issue Feb 2, 2021
…riptionsRepository

The MemorySubscriptionsRepository implementation does not synchronise
write access to the subscriptions list. As a result, Iterations over the
list will break when another Thread modifies the list.
This changes the ArrayList to a ConcurrentSkipListSet and changes the
interface of the public List<Subscription> listAllSubscriptions() method
to return a Set.
@hylkevds
Copy link
Collaborator Author

hylkevds commented Feb 3, 2021

PR is here: #572

@andsel andsel closed this as completed in 055031d May 2, 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

1 participant