Bug #3496
closed
WritePendingException from TestGetAsyncThreadPool on Windows
0%
Description
On Windows 7 (in a Parallels virtual machine) I run the following commands
mvn install
cd examples
mvn test -DclassName=TestGetAsyncThreadPool
It gives the following output (partially truncated):
net.named_data.jndn.transport.AsyncTcpTransport$2 completed
SEVERE: null
java.nio.channels.WritePendingException
at sun.nio.ch.AsynchronousSocketChannelImpl.write(AsynchronousSocketChannelImpl.java:352)
at sun.nio.ch.AsynchronousSocketChannelImpl.write(AsynchronousSocketChannelImpl.java:387)
at net.named_data.jndn.transport.AsyncTcpTransport.send(AsyncTcpTransport.java:223)
at net.named_data.jndn.Node.expressInterestHelper(Node.java:542)
at net.named_data.jndn.Node.access$000(Node.java:49)
at net.named_data.jndn.Node$1.run(Node.java:117)
at net.named_data.jndn.Node$2.run(Node.java:133)
at net.named_data.jndn.transport.AsyncTcpTransport$2.completed(AsyncTcpTransport.java:180)
at net.named_data.jndn.transport.AsyncTcpTransport$2.completed(AsyncTcpTransport.java:175)
at sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:126)
at sun.nio.ch.Invoker$2.run(Invoker.java:218)
at sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:112)
Time out for interest /
Updated by Anonymous over 8 years ago
WritePendingException is "thrown when an attempt is made to write to an asynchronous socket channel and a previous write has not completed."
https://docs.oracle.com/javase/7/docs/api/java/nio/channels/WritePendingException.html
Updated by Anonymous over 8 years ago
Hi Andrew. The previous notes for AsyncTcpTransport.send said "write is already async, so no need to dispatch." But I think that's not quite right and is causing the problem. I updated to the following comment:
https://github.com/named-data/jndn/blob/8a0157b12b3dfc7aabccca1c85205a8935e8d297/src/net/named_data/jndn/transport/AsyncTcpTransport.java#L238
Each call to AsynchronousSocketChannel.write dispatches to one of its pool threads, but there's no way to know which one. I think we got unlucky with the Windows implementation that dispatches to different threads which try to simultaneously write which throws WritePendingException. (You'd think that the whole point of an asynchronous communication library would be to handle such things but....)
Do I have to implement a send queue? Any ideas?
Updated by Anonymous over 8 years ago
... One of the top Google hits is the following where someone had to do just that - implement a send queue. Surely the Java library developers knew of this problem and provided a better built-in solution? (Resisting urge to weep...)
http://www.codeproject.com/Tips/766107/AsynchronousSocketChannel-Concurrent-Writes
Updated by Anonymous over 8 years ago
- Status changed from New to In Progress
Hi again Andrew. I put some code in the issue/3496-WritePendingException branch which works for me:
https://github.com/named-data/jndn/commit/23dc1c67a28e866c9999f55a3cdf6d1fa0c36ee1
Since AsynchronousSocketChannel.write dispatches to another thread, we don't need a send queue. We only need calls to AsyncTcpTransport.send to wait until the other thread is finished writing. I used an isWriting_ flag and use wait() and notify() to coordinate with the write completion handler which clears the flag when done. I hope this is "Java-like."
Can you try pull the issue/3496-WritePendingException branch and try it?
Updated by Andrew Brown over 8 years ago
I tried your fix and it seemed to work; the recommended practice in Java is to wrap wait()/notify() with one of the concurrency utilities. I used Lock; can I push a commit to this same branch demonstrating this and we can revert/rebase if necessary?
Updated by Anonymous over 8 years ago
Yes, push a change to demonstrate it. The wait/notify is already inside "synchronized (writingLock_)". If there is yet another layer of locks, then is the synchronized necessary?
Updated by Andrew Brown over 8 years ago
Added as a PR https://github.com/named-data/jndn/pull/10 so I could annotate the code; the test from the description worked for me (with Interest names altered).
Updated by Anonymous over 8 years ago
- Status changed from In Progress to Closed
Andrews pull request is tested and merged to master. Thanks!