Return-Path: X-Original-To: apmail-tomcat-dev-archive@www.apache.org Delivered-To: apmail-tomcat-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 023C09E17 for ; Thu, 1 Mar 2012 22:53:55 +0000 (UTC) Received: (qmail 73257 invoked by uid 500); 1 Mar 2012 22:53:54 -0000 Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org Received: (qmail 73202 invoked by uid 500); 1 Mar 2012 22:53:54 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 73188 invoked by uid 99); 1 Mar 2012 22:53:54 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 01 Mar 2012 22:53:54 +0000 X-ASF-Spam-Status: No, hits=-0.0 required=5.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: local policy) Received: from [66.160.196.165] (HELO arizona.hanik.com) (66.160.196.165) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 01 Mar 2012 22:53:46 +0000 Received: from [192.168.3.43] (71-218-153-18.hlrn.qwest.net [71.218.153.18]) by arizona.hanik.com (Postfix) with ESMTPSA id EB62D7830002 for ; Thu, 1 Mar 2012 15:50:59 -0700 (MST) Message-ID: <4F4FFE77.1010908@hanik.com> Date: Thu, 01 Mar 2012 15:55:51 -0700 From: Filip Hanik - Dev Lists User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0) Gecko/20111222 Thunderbird/9.0.1 MIME-Version: 1.0 To: Tomcat Developers List Subject: Re: WebSocket status References: <4F4CB5A4.3040104@apache.org> <4F4D07B4.6040201@hanik.com> <4F4D1F6D.10004@apache.org> <4F4FB98A.9050104@hanik.com> <4F4FBCDE.2030406@apache.org> In-Reply-To: <4F4FBCDE.2030406@apache.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Checked: Checked by ClamAV on apache.org 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 socket, Processor 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 { > @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-unsubscribe@tomcat.apache.org >> For additional commands, e-mail: dev-help@tomcat.apache.org >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org > For additional commands, e-mail: dev-help@tomcat.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org