Re: svn commit: r1513919 - in /tomcat/trunk: java/org/apache/catalina/connector/CoyoteAdapter.java test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

2013-08-15 Thread Peter Rossbach 
HI Mark,

nice fix :-) Thanks!

Why the testcase at NBReadServlet must called the listener.onDataAvailable();?

I read the 3.1 spec and think the container must call onDataAvailable and
we do that at CoyoteAdapter.

§3.7 P 3.28
■ onDataAvailable(). The onDataAvailable method is invoked on the 
ReadListener when data is available to read from the incoming request 
stream. The container will invoke the method the first time when data is 
available to read. The container will subsequently invoke the onDataAvailable
method if and only if isReady method on ServletInputStream, described 
below, returns false. 

But this simple POST example at 
https://weblogs.java.net/blog/swchan2/archive/2013/04/16/non-blocking-io-servlet-31-example
don't work.

Another question is: At which time the container must call the 
WriteListener.onWritePossible()?
After ReadListener.onDataAvailable is finished?

Currently we call onWritePossible before onDataAvailable is WriteListener set. 
The example set
WriteListener at the ReadListener has read all data! But the problem seems that 
no date available after POST request is dispatch.
After ReadListener set the method call result from ServletInputStream.isReady() 
is true! Seems socket has read data.

Regards
Peter

 
Am 14.08.2013 um 17:02 schrieb ma...@apache.org:

 Author: markt
 Date: Wed Aug 14 15:02:59 2013
 New Revision: 1513919
 
 URL: http://svn.apache.org/r1513919
 Log:
 Fix non-blocking test failures on OSX.
 
 Modified:
tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
 
 Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
 URL: 
 http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1513919r1=1513918r2=1513919view=diff
 ==
 --- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java 
 (original)
 +++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Wed 
 Aug 14 15:02:59 2013
 @@ -363,6 +363,10 @@ public class CoyoteAdapter implements Ad
 try {
 Thread.currentThread().setContextClassLoader(newCL);
 res.onWritePossible();
 +} catch (Throwable t) {
 +ExceptionUtils.handleThrowable(t);
 +res.getWriteListener().onError(t);
 +return false;
 } finally {
 Thread.currentThread().setContextClassLoader(oldCL);
 }
 @@ -379,6 +383,10 @@ public class CoyoteAdapter implements Ad
 if (request.isFinished()) {
 req.getReadListener().onAllDataRead();
 }
 +} catch (Throwable t) {
 +ExceptionUtils.handleThrowable(t);
 +req.getReadListener().onError(t);
 +return false;
 } finally {
 Thread.currentThread().setContextClassLoader(oldCL);
 }
 
 Modified: 
 tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
 URL: 
 http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java?rev=1513919r1=1513918r2=1513919view=diff
 ==
 --- tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java 
 (original)
 +++ tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java 
 Wed Aug 14 15:02:59 2013
 @@ -482,25 +482,20 @@ public class TestNonBlockingAPI extends 
 }
 
 @Override
 -public void onDataAvailable() {
 -try {
 -ServletInputStream in = ctx.getRequest().getInputStream();
 -String s = ;
 -byte[] b = new byte[8192];
 -int read = 0;
 -do {
 -read = in.read(b);
 -if (read == -1) {
 -break;
 -}
 -s += new String(b, 0, read);
 -} while (in.isReady());
 -log.info(s);
 -body.append(s);
 -} catch (Exception x) {
 -x.printStackTrace();
 -ctx.complete();
 -}
 +public void onDataAvailable() throws IOException {
 +ServletInputStream in = ctx.getRequest().getInputStream();
 +String s = ;
 +byte[] b = new byte[8192];
 +int read = 0;
 +do {
 +read = in.read(b);
 +if (read == -1) {
 +break;
 +}
 +s += new String(b, 0, read);
 +} while (in.isReady());
 +

Re: svn commit: r1513919 - in /tomcat/trunk: java/org/apache/catalina/connector/CoyoteAdapter.java test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

2013-08-15 Thread Mark Thomas 
On 15/08/2013 08:53, Peter Rossbach wrote:
 HI Mark,
 
 nice fix :-) Thanks!
 
 Why the testcase at NBReadServlet must called the 
 listener.onDataAvailable();?

https://issues.apache.org/bugzilla/show_bug.cgi?id=55381

 I read the 3.1 spec and think the container must call onDataAvailable and
 we do that at CoyoteAdapter.

We do, but that doesn't get triggered for the first call.

I have a patch for BZ55381 that works for BIO and NIO but not APR. I
know what the problem is - I just need to find a way around it. Oh, and
I have only tested on Windows so far. From experience, once this works
on Windows I'll need a few more tweaks to get it working on Linux and
OSX as well.

 Another question is: At which time the container must call the 
 WriteListener.onWritePossible()?
 After ReadListener.onDataAvailable is finished?

The specification doesn't define an order (there was some discussion in
the EG about what made sense) so applications should not assume any order.

 Currently we call onWritePossible before onDataAvailable is WriteListener 
 set. The example set
 WriteListener at the ReadListener has read all data! But the problem seems 
 that no date available after POST request is dispatch.
 After ReadListener set the method call result from 
 ServletInputStream.isReady() is true! Seems socket has read data.

I'm not sure I follow this. onWritePossible() and onDataAvailable() will
be called in response to a Poller event (for NIO and APR anyway). If
both a read and write event occurs, the poller will trigger an event for
each. Those events will trigger creation of a socket processor each
which gets passed to the executor. The executor will run them both. The
order of which one fires first depends on which on enters the sync block
first and that is not deterministic.

BIO is a whole different issue. I suspect that there will be some
applications that just don't work with BIO. Longer term, I'm wondering
about dropping BIO entirely.

Mark


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

svn commit: r1513919 - in /tomcat/trunk: java/org/apache/catalina/connector/CoyoteAdapter.java test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

2013-08-14 Thread markt 
Author: markt
Date: Wed Aug 14 15:02:59 2013
New Revision: 1513919

URL: http://svn.apache.org/r1513919
Log:
Fix non-blocking test failures on OSX.

Modified:
tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1513919r1=1513918r2=1513919view=diff
==
--- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Wed Aug 
14 15:02:59 2013
@@ -363,6 +363,10 @@ public class CoyoteAdapter implements Ad
 try {
 Thread.currentThread().setContextClassLoader(newCL);
 res.onWritePossible();
+} catch (Throwable t) {
+ExceptionUtils.handleThrowable(t);
+res.getWriteListener().onError(t);
+return false;
 } finally {
 Thread.currentThread().setContextClassLoader(oldCL);
 }
@@ -379,6 +383,10 @@ public class CoyoteAdapter implements Ad
 if (request.isFinished()) {
 req.getReadListener().onAllDataRead();
 }
+} catch (Throwable t) {
+ExceptionUtils.handleThrowable(t);
+req.getReadListener().onError(t);
+return false;
 } finally {
 Thread.currentThread().setContextClassLoader(oldCL);
 }

Modified: 
tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java?rev=1513919r1=1513918r2=1513919view=diff
==
--- tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java 
(original)
+++ tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java 
Wed Aug 14 15:02:59 2013
@@ -482,25 +482,20 @@ public class TestNonBlockingAPI extends 
 }
 
 @Override
-public void onDataAvailable() {
-try {
-ServletInputStream in = ctx.getRequest().getInputStream();
-String s = ;
-byte[] b = new byte[8192];
-int read = 0;
-do {
-read = in.read(b);
-if (read == -1) {
-break;
-}
-s += new String(b, 0, read);
-} while (in.isReady());
-log.info(s);
-body.append(s);
-} catch (Exception x) {
-x.printStackTrace();
-ctx.complete();
-}
+public void onDataAvailable() throws IOException {
+ServletInputStream in = ctx.getRequest().getInputStream();
+String s = ;
+byte[] b = new byte[8192];
+int read = 0;
+do {
+read = in.read(b);
+if (read == -1) {
+break;
+}
+s += new String(b, 0, read);
+} while (in.isReady());
+log.info(s);
+body.append(s);
 }
 
 @Override
@@ -537,35 +532,30 @@ public class TestNonBlockingAPI extends 
 }
 
 @Override
-public void onWritePossible() {
-try {
-long start = System.currentTimeMillis();
-long end = System.currentTimeMillis();
-int before = written;
-while (written  WRITE_SIZE 
-ctx.getResponse().getOutputStream().isReady()) {
-ctx.getResponse().getOutputStream().write(
-DATA, written, CHUNK_SIZE);
-written += CHUNK_SIZE;
-}
-if (written == WRITE_SIZE) {
-// Clear the output buffer else data may be lost when
-// calling complete
-ctx.getResponse().flushBuffer();
-}
-log.info(Write took: + (end - start) +
- ms. Bytes before= + before +  after= + written);
-// only call complete if we have emptied the buffer
-if (ctx.getResponse().getOutputStream().isReady() 
-written == WRITE_SIZE) {
-// it is illegal to call complete
-// if there is a write in progress
-