gary109 / barchart-udt

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

UDTInputStream and UDTOutputStream #17

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hey Andrei- I've just been working on my build environment in XCode,
but that's looking good now. UDTInputStream and UDTOutputStream will
definitely be easy to integrate into UDT proper, and I just need to
test them more.

-Adam

Original issue reported on code.google.com by Andrei.Pozolotin on 25 Jan 2011 at 2:59

GoogleCodeExporter commented 9 years ago
I'll integrate these as soon as I've got my dev environment fully up and have 
done more thorough testing.

Original comment by adamf...@gmail.com on 25 Jan 2011 at 7:30

GoogleCodeExporter commented 9 years ago
thank you.

Original comment by Andrei.Pozolotin on 25 Jan 2011 at 7:45

GoogleCodeExporter commented 9 years ago
I've got these pretty much all set, Andrei, and I added a test for them. They 
don't support all the InputStream and OutputStream methods (mark and reset, for 
example), but they should work for the vast majority of cases.

You want me to just go ahead and commit them?

Original comment by adamf...@gmail.com on 31 Jan 2011 at 7:16

GoogleCodeExporter commented 9 years ago
yes, please; do you mind if I try to "improve, if I see something?" ;-)

Original comment by Andrei.Pozolotin on 31 Jan 2011 at 2:49

GoogleCodeExporter commented 9 years ago
Definitely no problem on making improvements. I need to still integrate these 
with the Socket getInputStream and getOutputStream methods, and then I'll 
commit.

Original comment by adamf...@gmail.com on 31 Jan 2011 at 4:54

GoogleCodeExporter commented 9 years ago
Apologies for the delay, Andrei. I ran into what seems like a bug somewhere 
with the InputStreamUDT read() method -- the one that just reads a single byte. 
I'm still not quite sure what's happening, but it looks like it could be a 
deeper inconsistency somewhere on the C++ side.

If you enable the second test in 

barchart-udt4/src/test/java/com/barchart/udt/net/TestUdtInputAndOutputStream.jav
a

you should see it fail. Any thoughts on that?

Original comment by adamf...@gmail.com on 8 Feb 2011 at 8:18

GoogleCodeExporter commented 9 years ago
got it; checking;

Original comment by Andrei.Pozolotin on 8 Feb 2011 at 10:35

GoogleCodeExporter commented 9 years ago
The weird thing is this:

The test creates a server socket and a client socket. It then writes test bytes 
to the client socket. Then, the server socket side tries to read them, but it 
eventually hangs and cannot read another byte. Not sure why - the odd part is 
the hanging that starts on line 261 in echo() -- within copy the server thread 
eventually hangs on a read even though seemingly the client side has written 
all of the bytes.

Original comment by adamf...@gmail.com on 8 Feb 2011 at 11:04

GoogleCodeExporter commented 9 years ago
1) I re-wrote streams and test:
InputStreamUDT_2
OutputStreamUDT_2
TestStreamUDT_2

2) same deadlock in the test after reading 3 bytes;

3) this is due to some logic error in the test itself:
both client and server are waiting on each other - here is stack trace:

2011-02-08 22:19:38

"### Server-Echo-Thread" - Thread t@11
   java.lang.Thread.State: RUNNABLE
    at com.barchart.udt.SocketUDT.receive1(Native Method)
    at com.barchart.udt.SocketUDT.receive(SocketUDT.java:532)
    at com.barchart.udt.net.InputStreamUDT_2.read(InputStreamUDT_2.java:92)
    at com.barchart.udt.net.InputStreamUDT_2.read(InputStreamUDT_2.java:68)
    at com.barchart.udt.net.InputStreamUDT_2.read(InputStreamUDT_2.java:57)
    at com.barchart.udt.net.TestStreamUDT_2$2.read(TestStreamUDT_2.java:50)
    at com.barchart.udt.net.TestStreamUDT_2.copy(TestStreamUDT_2.java:209)
    at com.barchart.udt.net.TestStreamUDT_2.access$3(TestStreamUDT_2.java:175)
    at com.barchart.udt.net.TestStreamUDT_2$4.run(TestStreamUDT_2.java:307)
    at java.lang.Thread.run(Thread.java:662)

   Locked ownable synchronizers:
    - None

"### main" - Thread t@1
   java.lang.Thread.State: RUNNABLE
    at com.barchart.udt.SocketUDT.receive1(Native Method)
    at com.barchart.udt.SocketUDT.receive(SocketUDT.java:532)
    at com.barchart.udt.net.InputStreamUDT_2.read(InputStreamUDT_2.java:92)
    at com.barchart.udt.net.InputStreamUDT_2.read(InputStreamUDT_2.java:68)
    at com.barchart.udt.net.InputStreamUDT_2.read(InputStreamUDT_2.java:57)
    at com.barchart.udt.net.TestStreamUDT_2$2.read(TestStreamUDT_2.java:50)
    at com.barchart.udt.net.TestStreamUDT_2.copy(TestStreamUDT_2.java:209)
    at com.barchart.udt.net.TestStreamUDT_2.genericInputOutputTest(TestStreamUDT_2.java:119)
    at com.barchart.udt.net.TestStreamUDT_2.testSingleRead(TestStreamUDT_2.java:78)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
    at org.junit.runners.BlockJUnit4ClassRunner.runNotIgnored(BlockJUnit4ClassRunner.java:79)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:71)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:49)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:49)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

   Locked ownable synchronizers:
    - None

Original comment by Andrei.Pozolotin on 9 Feb 2011 at 4:23

GoogleCodeExporter commented 9 years ago
what I suggest to do next is write the test itself completely different, since 
both of us can not find anything wrong with it

Original comment by Andrei.Pozolotin on 9 Feb 2011 at 4:24

GoogleCodeExporter commented 9 years ago
also, it seems you need your streams to operate in blocking mode only?

in this case I suggest to use streams derived directly from sockets,
and not to use channels (take a look on InputStreamUDT_2 OutputStreamUDT_2);

channels are needed for "real nio" with selectors, keys, etc.

it is just more work to derive streams from channels
and make sure they will work correct in all permutations with selectors;

Original comment by Andrei.Pozolotin on 9 Feb 2011 at 4:28

GoogleCodeExporter commented 9 years ago
Hi Andrei- I agree on trying a different test technique. On the thread issue, 
though, the threads aren't waiting on each other -- that's the weird part. 

The main thread is certainly waiting on Server-Echo-Thread, but 
Server-Echo-Thread is trying to read the data that's already been written on 
the main thread -- the main thread with the trace you printed has already 
written all the data to the server at that point. The client is *only reading* 
at that point -- it's already done all it's writing -- so the server side can't 
be blocked on the client side writing.

So the real question is why the server side isn't getting any more data. I'll 
keep fiddling.

Thumbs up on the switch to not use channels -- definitely makes sense. Thanks 
Andrei.

Original comment by adamf...@gmail.com on 10 Feb 2011 at 5:16

GoogleCodeExporter commented 9 years ago
yes, they are not blocked in java thread sense, 
but they are both wating on "receive1",
which client should not do at all, I presume?

Original comment by Andrei.Pozolotin on 10 Feb 2011 at 4:24

GoogleCodeExporter commented 9 years ago
I moved / renamed stream classes and tests around (to seprate nio vs net); 
I hope you will find it OK;

Original comment by Andrei.Pozolotin on 10 Feb 2011 at 5:55

GoogleCodeExporter commented 9 years ago
Sounds good on the renaming front, Andrei.

As far as the blocking goes, how else would the client do reads if not through 
receive1? Should it use a different receive method?

Original comment by adamf...@gmail.com on 11 Feb 2011 at 2:15

GoogleCodeExporter commented 9 years ago
I commited new set of tests: see TestStreamBase;
both of your original use cases work (in different incarnation);

I guess now you have go back to your orginal test and make it work?

:-)

Original comment by Andrei.Pozolotin on 11 Feb 2011 at 3:56

GoogleCodeExporter commented 9 years ago
also note that streams is a short term plug;

if you really want to push to the limit, you must use nio;

you may want to take a look on mina:
http://mina.apache.org/

which allows to plugin udt selector provder and also possibly 
help you manage all your different little shoot protocols

Original comment by Andrei.Pozolotin on 11 Feb 2011 at 4:12

GoogleCodeExporter commented 9 years ago
Hey Andrei- I'm still a bit concerned about the underlying code. I just 
committed a change to my test that makes the issue clear. It allows you to 
switch between UDT and regular TCP sockets. The TCP version works while the UDT 
doesn't -- which is just not good.

Regarding NIO, LittleShoot uses NIO all over the place. I've written several of 
my own NIO libraries, have used MINA a bunch, and use Netty for anything new I 
do now -- it outperforms MINA by a lot. I did write LittleShoot SIP, STUN, and 
TURN clients and servers using MINA though. The section of the code that uses 
UDT needs Sockets for compatibility with HTTP client.

I'll have a look at your test case -- thanks. 

Original comment by adamf...@gmail.com on 11 Feb 2011 at 5:26

GoogleCodeExporter commented 9 years ago
Also, Andrei, not that streams are fundamentally incompatible with NIO from the 
Java API level. You'll get the same exceptions I'm throwing if you try to use 
streams with NIO in normal Java land.

Original comment by adamf...@gmail.com on 11 Feb 2011 at 5:27

GoogleCodeExporter commented 9 years ago
1) "TCP version works while the UDT doesn't" - I will take a look;

2) "Regarding NIO, LittleShoot uses NIO all over the place" - very cool;

Original comment by Andrei.Pozolotin on 11 Feb 2011 at 3:01

GoogleCodeExporter commented 9 years ago
1) problem solved: I spend too much time thinking in terms of buffers and 
channels, and streams just make no sense any more :-)

the only change needed is from TypeUDT.DATAGRAM into TypeUDT.STREAM

2) see TestStreamUDT

3) reason TypeUDT.DATAGRAM is no good for your test is here:
http://udt.sourceforge.net/udt4/doc/sendmsg.htm
"On the other hand, if the buf is not enough to hold the first message, only 
part of the message will be copied into the buffer, but the message will still 
be discarded after this recvmsg call."

4) before you jump and enforce the use of TypeUDT.STREAM in the streams wrappers
- think again -  but you are the customer 

:-)

Original comment by Andrei.Pozolotin on 11 Feb 2011 at 5:46

GoogleCodeExporter commented 9 years ago
correct link:
http://udt.sourceforge.net/udt4/doc/recvmsg.htm

Original comment by Andrei.Pozolotin on 11 Feb 2011 at 5:48

GoogleCodeExporter commented 9 years ago
This is really great Andrei -- nice catch. I just made that switch in my test, 
and it fixed it straight away.

I'm thinking about doing a few refactorings here -- wondering what you think. I 
still need a subclass of Socket, and it strikes me that one option would be to 
make AdapterSocketUDT public. It also strikes me the AdapterSocketUDT 
constructor could just take SocketUDT instead of also the channel, especially 
with associated refactorings to InputStreamUDT and OutputStreamUDT to use use 
SocketUDT directly instead of Channels.

So, this would basically make all the stuff I've been doing use SocketUDT 
instead of Channels, make AdapterSocketUDT a public class, and possibly tweak 
AdapterSocketUDT to only take a SocketUDT.

What do you think?

Original comment by adamf...@gmail.com on 13 Feb 2011 at 6:42

GoogleCodeExporter commented 9 years ago
I'd rather have it the way around (jdk keeps net-over-nio hidden); 

please take a look on commit 4197 if you like it?

Original comment by Andrei.Pozolotin on 13 Feb 2011 at 7:18

GoogleCodeExporter commented 9 years ago
Looks great to me, Andrei. Certainly works. I don't necessarily think the 
AdapterSocketUDT layer is necessary (why include channels at all?), but I like 
NetSocketUDT and NetServerSocketUDT and the new naming.

What do you think about having NetSocketUDT and NetServerSocketUDT construct 
their own SocketUDT with new new SocketUDT(TypeUDT.STREAM); instead of passing 
in the SocketUDT? Do you see cases where the SocketUDT would not be 
TypeUDT.STREAM?

Otherwise, definitely works for my purposes, and I really appreciate the 
effort! This is going into some crazy stuff (meeting with people from the State 
Department in DC about the circumvention tool this will be a part of), so I'll 
be sure to link back to this and plenty of credit!

Original comment by adamf...@gmail.com on 14 Feb 2011 at 1:24

GoogleCodeExporter commented 9 years ago
Did you push up the latest SNAPSHOT with those changes, Andrei? They definitely 
work on my end, so psyched to fully integrate them.

Original comment by adamf...@gmail.com on 14 Feb 2011 at 2:39

GoogleCodeExporter commented 9 years ago
1) re: "AdapterSocketUDT layer is necessary?" - it is needed for channels 
sockets "net-over-nio", 
since they have different connect/accept logic; it is not needed for pure-net 
sockets/streams;

2) re: "construct their own SocketUDT" - I added default constructors;
see if you like them;

3) re: "cases where the SocketUDT would not be TypeUDT.STREAM?" - not at the 
moment;
but I have seen enough limitations in jdk when someone was too sure that he is 
right
and made things private while I would be happy to use them;

4) re: "circumvention tool" - circumvention of what? :-)

5) re: "push up the latest SNAPSHOT" - I published the snapshot; with all the 
delays, etc
it now takes over 30 minutes to build all 6 platforms; we need to speed up 
tests,
make them delay-free; macosx is slowest and tests which are based on time logic
often fail because delays there are unreanable; again, must remove delay-based 
logic from tests;

6) re: "on my end, so psyched" - I am still finding little bugs here and there,
so dont get too disappointed if you hit some new wall somewhere;

Original comment by Andrei.Pozolotin on 14 Feb 2011 at 4:12

GoogleCodeExporter commented 9 years ago
All sounds great, Andrei, and I definitely like the new changes. 

On the tests with timing front, I completely agree. I just removed the 
now-redundant TestUdtInputAndOutputStream.java class, and I cleaned up the 
timing and threading in TestStreamUDT.java to use Thread.yield() instead of 
straight sleeps. It's super quick now, and I think it should always succeed. 
Let me know if you ever start seeing failures with it, though, and I'll revisit.

Thanks on the SNAPSHOT, and the circumvention tool is for circumventing censors 
and monitoring in countries where the government censors the Internet like 
China, Iran, Burma, etc.

I'll be doing a lot more testing on my end, so I'll keep an eye out for bugs.

Thanks again.

Original comment by adamf...@gmail.com on 14 Feb 2011 at 4:56

GoogleCodeExporter commented 9 years ago
cool, thanks; probably can close this.

Original comment by Andrei.Pozolotin on 14 Feb 2011 at 5:30

GoogleCodeExporter commented 9 years ago

Original comment by adamf...@gmail.com on 14 Feb 2011 at 5:43