Fix race in AioEventLoopGroup #662
Conversation
This reverts commit b39c1d5.
This reverts commit d201e3f.
shutdown. Is this really the right thing to do? If an EventLoop is shutdown shouldn't we just close all it associated channels instead of executing their I/O events by calling command.run()? The JDK documentation for AsynchronousChannelGroup.withThreadPool says: "Care should be taken when configuring the executor service. It should support direct handoff or unbounded queuing of submitted tasks, and the thread that invokes the execute method should never invoke the task directly. An implementation may mandate additional constraints."
|
I think you are right.. directly running the "Task" in the calling Thread is not good. I think if it was really shutdown we should throw an RejectedExecutionException. So I think it would be better if we check with "isShutdown()" if true we will throw the RejectedExecutionException. If not we try to execute the Tasks. This may also lead to the RejectedExecutionException but this is ok. WDYT ? |
|
If you want to go that way, you do not need the isShutdown check at all. Just call l.execute. But in case the EventLoop is shutdown and a RejectedExecutionException is thrown, who is supposed to receive that Exception? The calling Thread is an underlying "hidden" Thread from the AsynchronousChannelGroup. |
- Ensure the event loop threads are never terminated before all tasks submitted by JDK are executed - Close all open connections before terminating an event loop
|
c38c1d0 should fix the race condition completely, but at the moment I can't test it thoroughly because of MacOS X JDK issues. @alex-berger, would you mind if I ask you to test my commit and let me know it fixes your problem? |
- Fix a bug where shutdown() blocks indefinitely sometimes
|
Hi Trustin |
3 participants
The submission of a task after checking isShutdown on the EventLoop might fail as the EventLoop might be shutdown between the call to isShutdown and execute.
Anyhow, the whole thing feels a little bit warm and fuzzy to me. Is it really the right thing to do, to execute channel bound I/O events on the underlying AsynchronousChannelGroup's thread when the
channel's EventLoop is shutdown? The JDK API documentation for AsynchronousChannelGroup#withThreadPool clearly advises against such usage (see http://docs.oracle.com/javase/7/docs/api/java/nio/channels/AsynchronousChannelGroup.html#withThreadPool(java.util.concurrent.ExecutorService))
Should this really happen (Channel is still alive but its EventLoop is shutdown), wouldn't it be better to just close the Channel.