Closed GoogleCodeExporter closed 8 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
thank you.
Original comment by Andrei.Pozolotin
on 25 Jan 2011 at 7:45
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
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
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
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
got it; checking;
Original comment by Andrei.Pozolotin
on 8 Feb 2011 at 10:35
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
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
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
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
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
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
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
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
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
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
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
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
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
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
correct link:
http://udt.sourceforge.net/udt4/doc/recvmsg.htm
Original comment by Andrei.Pozolotin
on 11 Feb 2011 at 5:48
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
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
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
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
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
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
cool, thanks; probably can close this.
Original comment by Andrei.Pozolotin
on 14 Feb 2011 at 5:30
Original comment by adamf...@gmail.com
on 14 Feb 2011 at 5:43
Original issue reported on code.google.com by
Andrei.Pozolotin
on 25 Jan 2011 at 2:59