You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/08/14 17:02:59 UTC

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

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=1513919&r1=1513918&r2=1513919&view=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=1513919&r1=1513918&r2=1513919&view=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
-                    ctx.complete();
-                }
-            } catch (Exception x) {
-                x.printStackTrace();
+        public void onWritePossible() throws IOException {
+            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
+                ctx.complete();
             }
-
         }
 
         @Override



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


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

Posted by Mark Thomas <ma...@apache.org>.
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


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

Posted by Peter Rossbach <pr...@objektpark.de>.
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 markt@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=1513919&r1=1513918&r2=1513919&view=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=1513919&r1=1513918&r2=1513919&view=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
> -                    ctx.complete();
> -                }
> -            } catch (Exception x) {
> -                x.printStackTrace();
> +        public void onWritePossible() throws IOException {
> +            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
> +                ctx.complete();
>             }
> -
>         }
> 
>         @Override
> 
> 
> 
> ---------------------------------------------------------------------
> 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