hi Mark, sorry, I know you posted this earlier, thanks for reposting it to set
me straight.
Yes, it can make sense that you see the behavior you are. But in "theory" you
shouldn't.
If you call readSocket(false,bytes,off,maxRead), all the socket does is that it checks in the OS socket buffer to see if there is any data
available. If not, it immediately returns 0. This is the non blocking part of it. Now, since you call this part of a servlet, the socket is
set in zero interest mode (meaning, we don't want the socket poller to be reacting if data is coming in, this will cause concurrency issues
and poor performance)
So, what happens is that
1. You call read(block=false)
2. Your 1) call finishes, returns 0
3. Data arrives, ends up in the OS TCP receive buffer
4. You finish your "servlet request lifecycle" and return the socket to the
poller, with read interest
In step four, the poller should awake immediately for a read operation if there
is data in the.
However, what seems to be happening is a misuse of NIO in Http11NioProtocol
@Override
protected void upgradePoll(SocketWrapper<NioChannel> socket,
Processor<NioChannel> processor) {
connections.put(socket.getSocket(), processor);
SelectionKey key = socket.getSocket().getIOChannel().keyFor(
socket.getSocket().getPoller().getSelector());
key.interestOps(SelectionKey.OP_READ);
((KeyAttachment) socket).interestOps(
SelectionKey.OP_READ);
}
You can't register on a selector using another thread. This is most likely the cause of the problem, is the incorrect registration. You see,
you shouldn't be touching the poller from anywhere in the code itself. When the SocketProcessor returns, it needs to decide if the socket
goes back into the poller, for what operation, or if it needs to be cancelled/closed.
If you look at SocketProcessor.run method, the very last thing that is
happening here is
final SelectionKey fk = key;
final int intops = handshake;
final KeyAttachment ka = (KeyAttachment)fk.attachment();
ka.getPoller().add(socket,intops);
But my guess is that these new if/else clauses are bypassing this.
I'll work on a fix for this, and check it in so that you can see.
On a completely separate note, this WebSocket implementation seems to have caused a lot of duplicate code. I would have assumed the most
simple way to implement WebSockets would be on top of the existing Comet implementation. This implementation already generates the READ
event when data arrives, and you have the ability to check available() to see if there is more data to be read. This would too have avoided
single byte reads from the system buffers, since Comet reads in as much as possible, and then you can single byte read from Java heap memory.
The only thing you would have had to do, had you used Comet, was to turn off the filters that cause chunked-encoding, and you would have had
access to raw data directly from within. All the Upgrade classes, everything, would have sat directly in the highest level, never touching
the low level connectors.
Something food for thought.... :)
On 3/1/2012 11:15 AM, Mark Thomas wrote:
On 01/03/2012 18:01, Filip Hanik - Dev Lists wrote:
Alright, now that I'm all squared away with Autobahn and test cases
running. Is there a specific unit test you have to produce this behavior?
That would help me for digging through it.
Assuming you are running trunk, then what you'll currently have is NIO
is a purely blocking mode. The tests should all pass and take a minute
or two to run. If you apply the following patch, you'll enable
non-blocking reads for NIO at which point large numbers of tests should
start to fail / take forever / timeout. I usually just run the section 1
tests (i.e. "cases": ["1.*"]) in fuzzing_client_spec.json when
investigating these are they are mostly small payloads easy to trace.
HTH,
Mark
--- a/java/org/apache/coyote/http11/upgrade/UpgradeNioProcessor.java
+++ b/java/org/apache/coyote/http11/upgrade/UpgradeNioProcessor.java
@@ -104,12 +104,10 @@ public class UpgradeNioProcessor extends
UpgradeProcessor<NioChannel> {
@Override
public int read(boolean block, byte[] bytes, int off, int len)
throws IOException {
- // TODO Implement non-blocking reads. Should be as simple as
replacing
- // true with block in the two lines below
if (len> maxRead) {
- return readSocket(true, bytes, off, maxRead);
+ return readSocket(block, bytes, off, maxRead);
} else {
- return readSocket(true, bytes, off, len);
+ return readSocket(block, bytes, off, len);
}
}
Filip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
