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

ConcurrentModificationException of org.jeromq.ZMQ.Poller #68

Closed
kdlan opened this issue Jun 4, 2013 · 4 comments
Closed

ConcurrentModificationException of org.jeromq.ZMQ.Poller #68

kdlan opened this issue Jun 4, 2013 · 4 comments

Comments

@kdlan
Copy link

kdlan commented Jun 4, 2013

I got a ConcurrentModificationException when

java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
at java.util.HashMap$KeyIterator.next(HashMap.java:828)
at java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1008)
at zmq.ZMQ.zmq_poll(ZMQ.java:653)
at org.jeromq.ZMQ$Poller.poll(ZMQ.java:1488)

I have check the code
org.jeromq.ZMQ

The selector is hold in the ThreadLocal variable.

If I create a Poller in main thread and put it to a worker thread.

Then create another Poller in main thread and put it to another worker thread.

Both the tow Poller will use the same Selector

It will throw ConcurrentModificationException.

And a thread blocked on a register method in zmq.ZMQ.java

After I change my code to create the Poller in their own worker thread, The Exception never thrown again.

I wonder if this ThreadLocal holder is necessary. And there is lack of document to indicate this. Or it is just a jdk bug?

OS: ubuntu, kenerl 2.6.38 , x64
Java: oracle jdk 1.6.0_34
jeromq: 0.2.0

@miniway
Copy link
Member

miniway commented Jun 4, 2013

Basically, I don't like using ThreadLocal, but it was not easy to make API compatible with jzmq without performance penalty.

The first implementation was to create Selector at the poller constructor. This was safer but some usage and example pattern was

while
    create poller            # creates Selector every time and has severe performance penalty
    register handles
    poll

At that time, there was a document to guide to create a poller outside of a loop,

create poller
register handles
while
    poll

From the 0.2.0, ThreadLocal has been used as both patterns could have same performance based on assumptions.

  • Poller should be created and used on the same thread
  • There would not be a case which one thread needs to wait on multiple poller.

We should have enhanced the document.

@kdlan
Copy link
Author

kdlan commented Jun 4, 2013

But this still force me to change my code...

Have you considered to move the Poller.open() from the constructor to the Poller.poll() where the selector is eventually been used in the Poller. In this way, the Poller object shoud not need to hold the selector instance and when call Poller.poll, just get it from the ThreadLocal variable and pass it to zmq.ZMQ.zmq_poll

This seems can fix my issue and have no influence on performance as 0.2.0.

@miniway
Copy link
Member

miniway commented Jun 6, 2013

I would still recommend not sharing the Poller object between threads for a future change.

If you think the change is meaningful to the community, please send a pull request on it.

@kdlan
Copy link
Author

kdlan commented Jun 6, 2013

It seems 0.3.0-SNAPSHOT have deprecated org.jeromq.ZMQ and org.zeromq.ZMQ.Poller does not holder the threadlocal variable. I will wait 0.3.0 and move to the new api

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

3 participants