gary109 / barchart-udt

Automatically exported from code.google.com/p/barchart-udt
0 stars 0 forks source link

NETTY INTEGRATION --- connect() throws IllegalBlockingModeException when called on non-blocking, unregistered ChannelSocketUDT instances #21

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

1. Create a ChannelSocketUDT instance
2. Set it to non-blocking
3. Attempt to connect it to a remote host

What is the expected output? What do you see instead?

Expected output (based on java.nio.channels.SocketChannel contract and 
behaviour of JDK provided implementations) is a return value of false (or 
possibly true in unlikely circumstances), allowing use of finishConnect() at a 
later stage.

Instead an IllegalBlockingMode exception is thrown stating that UDT channel 
must be registered with a selector before connect() is used.

What version of the product are you using? On what operating system?

release 1.0.2 from maven, although the relevant classes have not changed as of 
r4123 in trunk

Please provide any additional information below.

I am attempting to integrate barchart-UDT into the Netty NIO framework 
(http://www.jboss.org/netty) by modifying/extending the existing TCP based 
transport within Netty. Netty takes the approach (with the TCP based transport) 
of creating a SocketChannel object, setting it to non-blocking and then calling 
connect(), deferring registration with a selector until the connection 
succeeds. Maybe this is to reduce resource overhead of failed connections? I am 
not sure.

Do you think this is possible to achieve within barchart-UDT? It would be great 
if the behaviour matched the Sun/Oracle SocketChannel implementations. It seems 
that the current design makes use of SelectorUDT and requires an associated 
SelectionKeyUDT before connect() is called.

My skills in NIO internals are somewhat lacking, but if there is not sufficient 
interest in addressing this I am prepared to attempt it, but would appreciate 
any suggestions on the approach. 

Great work all of you are doing on this project though, it is very excited to 
have the possibility of UDT available from Java! :-)

Original issue reported on code.google.com by willjc...@gmail.com on 25 Apr 2011 at 6:37

GoogleCodeExporter commented 9 years ago
hello!

1) please try this snapshot instead:
https://oss.sonatype.org/content/repositories/snapshots/com/barchart/udt/barchar
t-udt4-bundle/1.0.3-SNAPSHOT/

2) see if this unit test works for you:
http://code.google.com/p/barchart-udt/source/browse/trunk/barchart-udt4-parent/b
archart-udt4/src/test/java/com/barchart/udt/net/SimpleUdtTest.java

3) udt for netty: cool. I need that too. I asked Trustin Lee
https://issues.jboss.org/secure/ViewProfile.jspa?name=trustin
couple of times:
http://www.jboss.org/netty/community.html#nabble-td6186555
https://issues.jboss.org/browse/NETTY-392
got no answer so far; please add you voice to this;

4) barchart udt is impemented to be 100% compatible with sun nio 
channels/selectors spi 
(http://download.oracle.com/javase/6/docs/api/java/nio/channels/spi/SelectorProv
ider.html); netty adaptation should be easy.

in the current netty code Trustin is not using any spi, he goes straight for 
default sun provider; this needs to change on his side; or else we have to fork 
netty :-) (may be not, I have to take more close look) ; another problem is to 
ask Trustin to propagate extra config parameters to control if stream spi is 
making tcp or udt sockets and also custom properties for udt; 

cheers!

Original comment by Andrei.Pozolotin on 25 Apr 2011 at 9:51

GoogleCodeExporter commented 9 years ago
correct link:
http://download.oracle.com/javase/6/docs/api/java/nio/channels/spi/SelectorProvi
der.html

Original comment by Andrei.Pozolotin on 25 Apr 2011 at 9:56

GoogleCodeExporter commented 9 years ago
Hi Andrei,

Great, thanks for the links!

I am still getting the exception with the new 1.0.3 snapshot however:

java.nio.channels.IllegalBlockingModeException
    at com.barchart.udt.nio.ChannelSocketUDT.connect(ChannelSocketUDT.java:167)
    at org.jboss.netty.channel.socket.nio.NioClientSocketPipelineSinkUDT.connect(NioClientSocketPipelineSinkUDT.java:142)
    at org.jboss.netty.channel.socket.nio.NioClientSocketPipelineSinkUDT.eventSunk(NioClientSocketPipelineSinkUDT.java:105)
    at org.jboss.netty.channel.Channels.connect(Channels.java:541)
    at org.jboss.netty.channel.AbstractChannel.connect(AbstractChannel.java:218)
    at org.jboss.netty.bootstrap.ClientBootstrap.connect(ClientBootstrap.java:227)
    at org.jboss.netty.bootstrap.ClientBootstrap.connect(ClientBootstrap.java:188)
    at org.jboss.netty.example.echo.EchoClientUDT.main(EchoClientUDT.java:79)

The Netty approach seems to favour registering channels only once they
have connected, this 'lazy' approach may have performance advantages
or it may be fairly arbitrary, I do not understand the code well
enough to tell yet.

Whilst it looks like the ChannelSocketUDT.connect() method does not
exactly 'break' the contract given in [1], the behaviour is different
from that of the Sun SPI SocketChannelImpl class when a connect() is
attempted from a non-blocking instance that is *not registered* with
any Selector yet. The Sun SPI connect() returns (false normally) and
Netty sets up task to register the channel asynchronously. I think I
can work around it by changing the ordering in the 'UDT' transport for
Netty without needing to go too deep.

It seems, technically, ChannelSocketUDT may not in fact be breaking
the SocketChannel API since the last part of the Javadoc states that
connect() may throw "IOException - If some other I/O error occurs".
Its just that this causes the exception when used with code that does
the "connect first, register later" style with non-blocking already
configured. This includes Netty code as well as example code from the
O'Reilly 'Java NIO' book, it seems to be an idiom used in at least a
few places.

I'm assuming the corresponding code for the 1.0.3 snapshot is the same
as currently in trunk (I see no tag for 1.0.3), so I don't think the
behaviour will be different from 1.0.2.

I'll add my voice on the mailing list as Netty and this project seem
like a really good match. Trustin did express interest in adding
support for different SelectorProviders per ChannelFactory [2] but the
original enquirer seems to have done a pretty good job of talking him
out of it! In the worst case it will just mean maintaining a few Netty
custom transport classes, the SPI specific stuff does not get handled
outside of netty's  ...channel.socket.nio package. That said it would
be nice to be able to just pass in the desired provider to a generic
factory though :-)

I also raised a query [3] a few days ago but so far no reply either (I
have tried many 'reliable' UDP implementations in Java recently, this
one looks the most promising).

One other thing, it seems that ChannelServerSockeUDT.accept() should
return null if it is non-blocking mode and there are no queued
attempts from clients, according to [4]. I could post another issue
about that if you want.

cheers,

Will

[1] 
http://download.oracle.com/javase/6/docs/api/java/nio/channels/SocketChannel.htm
l#connect(java.net.SocketAddress)
[2] 
http://netty-forums-and-mailing-lists.685743.n2.nabble.com/using-custom-Selector
Providers-tp4536989p4536989.html;cid=1303660947964-563
[3] 
http://netty-forums-and-mailing-lists.685743.n2.nabble.com/Using-more-than-one-S
electorProvider-in-the-same-JVM-tp6287573p6287573.html;cid=1303660947964-563
[4] 
http://download.oracle.com/javase/6/docs/api/java/nio/channels/ServerSocketChann
el.html#accept()

Original comment by willjc...@gmail.com on 25 Apr 2011 at 11:29

GoogleCodeExporter commented 9 years ago
Will:

sorry, there is a deeper issue here :-)

1) providers, selectors, channels & keys share intimate relationships with each 
other; take a look on jdk 6 nio sources - both java and jni; or take a deeper 
look on my udt sources for selector provider;

2) as it stands now default sun jdk provider can work only with sun jdk channels
and my udt provider can work only with my udt channels; what you are trying to 
do seems logical, but it will not work;

3) original reason for this split was that way back original udt in c++ did not 
support epoll() and I had to emulate it; as a result now I have my own "netty" 
where I use both jdk an udt selectors at the same time in same jvm just fine; I 
just need to instaniate appropriate selectors for appropriate channels;

4) the easy way to put udt nio in netty now would be repeat this approach; yes 
it requires 2 selector providers in the same jvm; again, works fine for me for 
more then a year;

5) the hard way would be to go back and re-do udt selector implementation to 
"hack" it into current sun provider; or make a new provider which is combines 
them both; either way, Trustin need to add support for non-default and possibly 
multiple providers in netty;

so it seems you best contribution would be to call Trustin and inspire him so 
he can start paying some attention to this :-)

Andrei

Original comment by Andrei.Pozolotin on 26 Apr 2011 at 1:56

GoogleCodeExporter commented 9 years ago
Hi Andrei,

Yes indeed :-)

I'm not sure you understand my approach. This is what I have done:

* 'cloned' all the relevant netty TCP transport classes and renamed
them <class-name>UDT
* updated all relevant references to point to these new classes (using
refactoring tools)
* changed all uses of Sun SPI factory methods to use the UDT factory
methods in these new classes

    e.g. all instances of:
        this.selector = selector =  Selector.open();
    changed to:
        this.selector = selector = SelectorProviderUDT.STREAM.openSelector();

    all instances of:
        socket = SocketChannel.open();
    changed to:
        socket = SelectorProviderUDT.STREAM.openSocketChannel();

    and all instances of:
        socket = ServerSocketChannel.open();
   changed to:
        socket = SelectorProviderUDT.STREAM.openServerSocketChannel();

* make use of the new NioSocketChannelFactoryUDT and
NioServerSocketChannelFactoryUDT classes as drop in replacement of
Netty's TCP based ones.

This way I am sure that the only SelectorProvider SPI implementation
classes being used in my application are the ones from barchart-UDT (I
have checked extensively from breakpoints).  Can you see anything
wrong with this approach?

Interesting, are you able to share any of the code for the working
Netty setup? It might be a great help towards getting my (university)
project moving along a bit quicker, it has been stalled due to lack of
a reliable stream based NAT traversal solution for Java (hence why I
need a UDP based protocol) :-)

It isn't a problem for me to ensure that all UDT Channel and Selector
classes are isolated from other SPI implementations. The problem I
have is that Netty (amongst other code) takes the approach of
connecting a non-blocking channel and then registering it with a
selector afterwards. Like I said previously, I think I can work around
this with further modifications to the new Netty transport but it
seems that the ChannelSocketUDT.connect() behaviour is non-standard
and breaks code that attempts to use the UDT SPI classes as a drop in
replacement. Broken code includes the example NIO code from Sun and
the 'Java NIO' book from O'Reilly (see below).

So forgetting Netty, here is a simple example (no need to have a
server to connect to since the problem arises early enough):

package udt.test;

import java.net.InetSocketAddress;
import java.nio.channels.*;
import java.nio.channels.spi.SelectorProvider;

import com.barchart.udt.nio.*;

public class TestConnect {
    public static void main(String[] args) throws Exception {
        SelectorProvider sp;

        // causes IllegalBlockingException during connect() :
        sp = SelectorProviderUDT.STREAM;

        // causes connect() to return normally :
        //sp = SelectorProvider.provider();

        Selector sel = sp.openSelector();
        SocketChannel sc = sp.openSocketChannel();

        sc.configureBlocking(false);

                // throws IllegalBlockingException when using UDT provider
                sc.connect(new InetSocketAddress("127.0.0.1", 8090));
        sc.register(sel, SelectionKeyUDT.OP_CONNECT);

        while (true) {
            sel.select();
            for (SelectionKey sk : sel.selectedKeys()) {
                SocketChannel s = (SocketChannel) sk.channel();
                if (s.finishConnect())
                    { System.out.println("Connect succeeded :-)"); }
            }
        }   
    }
}

This works fine when sp is set to Sun's provider, but fails with the
UDT one. It does work fine when used with the UDT provider if the call
to register() is moved to before the connect() call, but there seems
to be a lot of code out there that does connect() first and register()
later. For example see:

http://download.oracle.com/javase/1.4.2/docs/guide/nio/example/Ping.java
http://www.javanio.info/filearea/bookexamples/unpacked/com/ronsoft/books/nio/cha
nnels/ConnectAsync.java
http://cs.ecs.baylor.edu/~donahoo/practical/JavaSockets2/code/TCPEchoClientNonbl
ocking.java
https://github.com/netty/netty/blob/master/src/main/java/org/jboss/netty/channel
/socket/nio/NioClientSocketPipelineSink.java#L130

Wouldn't it be good to have the same behaviour as Sun's implementation?

Looking at the barchart-UDT code it looks like the design relies on
channels being registered in order to initiate the underlying UDT
connection, since the SelectorKey is used in
SelectorUDT.submitConnectRequest() and also within
ConnectorThreadPoolUDT (as the lookup key for the map of channels). I
can see how this means that connecting a ChannelSocketUDT requires a
SelectorKey and hence for it to be pre-registered. Would it be better
for the SocketChannels to be mapped using some other key ( e.g.
Object.hashcode() ) for connection purposes? This would allow
unregistered channels to connect. I realise such a change is probably
not simple or easy.

I'd love to demonstrate a solution to this but I need time to get my
head around the lower level code and I have university deadlines
looming, so for now I will attempt to modify the (connect/register)
ordering within Netty. Feel free to shoot down what I have said about
the design. Its all learning to me, thanks for taking the time to
discuss this :-)

cheers, Will

Original comment by willjc...@gmail.com on 26 Apr 2011 at 11:02

GoogleCodeExporter commented 9 years ago
if you read the email version it should be less confusing (inline quotes have 
been removed from the google code view of this thread), apologies ;-)

Original comment by willjc...@gmail.com on 26 Apr 2011 at 11:13

GoogleCodeExporter commented 9 years ago
cool, got it; 
now I finally must run your test and see whats up; :-)
will be back then.

Original comment by Andrei.Pozolotin on 26 Apr 2011 at 2:18

GoogleCodeExporter commented 9 years ago
Hi, just to let you know, I have made the modification in my Netty UDT
transport, it appears to work fine from my initial tests but it did
need the additional fix inside ChannelServerSocketUDT that I mentioned
earlier. Netty freaked out because accept() method was not returning
null if there were no queued connection attempts (in-non blocking
mode), as detailed here:
http://download.oracle.com/javase/6/docs/api/java/nio/channels/ServerSocketChann
el.html#accept()

Here is the version of ChannelServerSocketUDT.accept() that I have come up with:

    @Override
    public SocketChannel accept() throws IOException {

        SocketUDT socketUDT = null;
        try {
            begin();
            socketUDT = serverSocketUDT.accept();
            if (!isBlocking() && socketUDT == null) {
                // non-blocking with no attempts received
                return null;
            } else {
                SelectorProvider provider = provider();
                return new ChannelSocketUDT(provider, socketUDT);
            }
        } finally {
            // indicate success/failure
            end(socketUDT == null);
        }
    }

It all seems to work fine at the moment, I've not tested under heavy
load/concurrency yet though :-)

Original comment by willjc...@gmail.com on 26 Apr 2011 at 4:52

GoogleCodeExporter commented 9 years ago
Hi, just to let you know, I have made the modification in my Netty UDT
transport, it appears to work fine from my initial tests but it did
need the additional fix inside ChannelServerSocketUDT that I mentioned
earlier. Netty freaked out because accept() method was not returning
null if there were no queued connection attempts (in-non blocking
mode), as detailed here:
http://download.oracle.com/javase/6/docs/api/java/nio/channels/ServerSocketChann
el.html#accept()

Here is the version of ChannelServerSocketUDT.accept() that I have come up with:

    @Override
    public SocketChannel accept() throws IOException {

        SocketUDT socketUDT = null;
        try {
            begin();
            socketUDT = serverSocketUDT.accept();
            if (!isBlocking() && socketUDT == null) {
                // non-blocking with no attempts received
                return null;
            } else {
                SelectorProvider provider = provider();
                return new ChannelSocketUDT(provider, socketUDT);
            }
        } finally {
            // indicate success/failure
            end(socketUDT == null);
        }
    }

It all seems to work fine at the moment, I've not tested under heavy
load/concurrency yet though :-)

Original comment by willjc...@gmail.com on 26 Apr 2011 at 4:55

GoogleCodeExporter commented 9 years ago
wow, man you are too fast :-)
let me know you gmail so I can add you as commiter here (so you can commit your 
fix yourself); also all fixes need to come with unit tests :-)

Original comment by Andrei.Pozolotin on 26 Apr 2011 at 7:01

GoogleCodeExporter commented 9 years ago
hehe, if you knew how many hours I've spent looking at NIO and UDP
based protocols/transports over the last week you may think
differently ;-)

Always happy to become a commiter  :-) it's willjcroz at <you know what>

I think I need to test the whole setup a bit more and look into the
unit tests before commiting the fix, this may take a while as I have a
lot of work on right now.

Also, under heavier testing (heavy connect/disconnect load from
multiple threads) I'm getting the occasional exception coming from
SocketUDT.send2() and receive2() due to a lost connection, it might be
my changes in Netty to workaround the connect/register ordering, needs
investigating further :

com.barchart.udt.ExceptionUDT: UDT Error : 2001 : connection was
broken : recv/recvmsg (socketID=88152316)
    at com.barchart.udt.SocketUDT.receive2(Native Method)
    at com.barchart.udt.SocketUDT.receive(SocketUDT.java:549)
    at com.barchart.udt.nio.ChannelSocketUDT.read(ChannelSocketUDT.java:248)
    at org.jboss.netty.channel.socket.nio.NioWorkerUDT.read(NioWorkerUDT.java:336)
    at org.jboss.netty.channel.socket.nio.NioWorkerUDT.processSelectedKeys(NioWorkerUDT.java:295)
    at org.jboss.netty.channel.socket.nio.NioWorkerUDT.run(NioWorkerUDT.java:215)
    at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
    at java.lang.Thread.run(Thread.java:662)

com.barchart.udt.ExceptionUDT: UDT Error : 2001 : connection was
broken : send/sendmsg (socketID=88152316)
    at com.barchart.udt.SocketUDT.send2(Native Method)
    at com.barchart.udt.SocketUDT.send(SocketUDT.java:802)
    at com.barchart.udt.nio.ChannelSocketUDT.write(ChannelSocketUDT.java:349)
    at org.jboss.netty.channel.socket.nio.SocketSendBufferPool$PooledSendBuffer.transferTo(SocketSendBufferPool.java:239)
    at org.jboss.netty.channel.socket.nio.NioWorkerUDT.write0(NioWorkerUDT.java:484)
    at org.jboss.netty.channel.socket.nio.NioWorkerUDT.writeFromUserCode(NioWorkerUDT.java:402)
    at org.jboss.netty.channel.socket.nio.NioClientSocketPipelineSinkUDT.eventSunk(NioClientSocketPipelineSinkUDT.java:119)
    at org.jboss.netty.channel.Channels.write(Channels.java:632)
    at org.jboss.netty.channel.Channels.write(Channels.java:593)

etc.........

hopefully nothing too serious :-)

Original comment by willjc...@gmail.com on 26 Apr 2011 at 10:43

GoogleCodeExporter commented 9 years ago
you are now a commiter

Original comment by Andrei.Pozolotin on 27 Apr 2011 at 2:05

GoogleCodeExporter commented 9 years ago
Thanks Andrei, I'll investigate the exception and let you know how I get on 
soon.

As for Netty integration, I think the way forward would be to modify Netty to 
allow specifying the SPI implementation during construction of the Netty 
channel factory, and then send Trustin a pull request on GitHub so that he can 
get an idea of what we need, even if he doesn't like the actual patch.

One other thing I was thinking about is Netty's use of constraint level as 
detailed here: 
https://github.com/netty/netty/blob/master/src/main/java/org/jboss/netty/channel
/socket/nio/NioProviderMetadata.java#L50

It may be related to the exceptions I'm getting under heavy connect/disconnect 
load.

Any idea what constraint level Barchart-UDT should be able to support? I'll 
look into testing this also.

Original comment by willjc...@gmail.com on 28 Apr 2011 at 11:17

GoogleCodeExporter commented 9 years ago
i'm facing the similar error:

com.barchart.udt.ExceptionUDT: UDT Error : 2001 : connection was broken : 
recv/recvmsg [id: 0x2c2ba18c]
    at com.barchart.udt.SocketUDT.receive1(Native Method)
    at com.barchart.udt.SocketUDT.receive(SocketUDT.java:1183)
    at com.barchart.udt.net.NetInputStreamUDT.read(NetInputStreamUDT.java:83)
    at com.barchart.udt.net.NetInputStreamUDT.read(NetInputStreamUDT.java:74)
    at com.barchart.udt.net.NetInputStreamUDT.read(NetInputStreamUDT.java:63)
    at java.io.DataInputStream.readInt(DataInputStream.java:387
    at ...

what could be the reason for that?

Original comment by andi...@gmail.com on 1 Apr 2014 at 2:12