You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by vi...@apache.org on 2016/10/21 11:21:48 UTC

svn commit: r1765995 - /tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Author: violetagg
Date: Fri Oct 21 11:21:48 2016
New Revision: 1765995

URL: http://svn.apache.org/viewvc?rev=1765995&view=rev
Log:
Test case for a delayed registration of WriteListener. Test is marked as @Ignore as it is failing with the current implementation.

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

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=1765995&r1=1765994&r2=1765995&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java (original)
+++ tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java Fri Oct 21 11:21:48 2016
@@ -23,9 +23,14 @@ import java.net.HttpURLConnection;
 import java.net.Socket;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import javax.net.SocketFactory;
 import javax.servlet.AsyncContext;
@@ -41,6 +46,7 @@ import javax.servlet.http.HttpServletReq
 import javax.servlet.http.HttpServletResponse;
 
 import org.junit.Assert;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import org.apache.catalina.Context;
@@ -769,4 +775,135 @@ public class TestNonBlockingAPI extends
         }
         return rc;
     }
+
+
+    @Ignore
+    @Test
+    public void testDelayedNBWrite() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        Context ctx = tomcat.addContext("", null);
+        CountDownLatch latch1 = new CountDownLatch(1);
+        DelayedNBWriteServlet servlet = new DelayedNBWriteServlet(latch1);
+        String servletName = DelayedNBWriteServlet.class.getName();
+        Tomcat.addServlet(ctx, servletName, servlet);
+        ctx.addServletMappingDecoded("/", servletName);
+
+        tomcat.start();
+
+        CountDownLatch latch2 = new CountDownLatch(2);
+        List<Throwable> exceptions = new ArrayList<>();
+
+        Thread t = new Thread(
+                new RequestExecutor("http://localhost:" + getPort() + "/", latch2, exceptions));
+        t.start();
+
+        latch1.await(3000, TimeUnit.MILLISECONDS);
+
+        Thread t1 = new Thread(new RequestExecutor(
+                "http://localhost:" + getPort() + "/?notify=true", latch2, exceptions));
+        t1.start();
+
+        latch2.await(3000, TimeUnit.MILLISECONDS);
+
+        if (exceptions.size() > 0) {
+            Assert.fail();
+        }
+    }
+
+    private static final class RequestExecutor implements Runnable {
+        private final String url;
+        private final CountDownLatch latch;
+        private final List<Throwable> exceptions;
+
+        public RequestExecutor(String url, CountDownLatch latch, List<Throwable> exceptions) {
+            this.url = url;
+            this.latch = latch;
+            this.exceptions = exceptions;
+        }
+
+        @Override
+        public void run() {
+            try {
+                ByteChunk result = new ByteChunk();
+                int rc = getUrl(url, result, null);
+                Assert.assertTrue(rc == HttpServletResponse.SC_OK);
+                Assert.assertTrue(result.toString().contains("OK"));
+            } catch (Throwable e) {
+                e.printStackTrace();
+                exceptions.add(e);
+            } finally {
+                latch.countDown();
+            }
+        }
+
+    }
+
+    @WebServlet(asyncSupported = true)
+    private static final class DelayedNBWriteServlet extends TesterServlet {
+        private static final long serialVersionUID = 1L;
+        private final Set<Emitter> emitters = new HashSet<>();
+        private final CountDownLatch latch;
+
+        public DelayedNBWriteServlet(CountDownLatch latch) {
+            this.latch = latch;
+        }
+
+        @Override
+        protected void doGet(HttpServletRequest request, HttpServletResponse response)
+                throws ServletException, IOException {
+            boolean notify = Boolean.valueOf(request.getParameter("notify"));
+            AsyncContext ctx = request.startAsync();
+            ctx.setTimeout(1000);
+            if (!notify) {
+                emitters.add(new Emitter(ctx));
+                latch.countDown();
+            } else {
+                for (Emitter e : emitters) {
+                    e.emit();
+                }
+                response.getOutputStream().println("OK");
+                response.getOutputStream().flush();
+                ctx.complete();
+            }
+        }
+
+    }
+
+    private static final class Emitter {
+        private final AsyncContext ctx;
+
+        Emitter(AsyncContext ctx) {
+            this.ctx = ctx;
+        }
+
+        void emit() throws IOException {
+            ctx.getResponse().getOutputStream().setWriteListener(new WriteListener() {
+                private boolean written = false;
+
+                @Override
+                public void onWritePossible() throws IOException {
+                    ServletOutputStream out = ctx.getResponse().getOutputStream();
+                    if (out.isReady() && !written) {
+                        out.println("OK");
+                        written = true;
+                    }
+                    if (out.isReady() && written) {
+                        out.flush();
+                        if (out.isReady()) {
+                            ctx.complete();
+                        }
+                    }
+                }
+
+                @Override
+                public void onError(Throwable t) {
+                    t.printStackTrace();
+                }
+
+            });
+        }
+
+    }
+
 }



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


Re: svn commit: r1765995 - /tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Violeta,

On 10/27/16 7:28 AM, Violeta Georgieva wrote:
> Hi,
> 
> 2016-10-27 13:53 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>>
>> 2016-10-23 8:22 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>>
>>> Hi Remy,
>>>
>>> 2016-10-21 15:11 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>>>>
>>>> 2016-10-21 13:21 GMT+02:00 <vi...@apache.org>:
>>>>
>>>>> Author: violetagg
>>>>> Date: Fri Oct 21 11:21:48 2016
>>>>> New Revision: 1765995
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1765995&view=rev
>>>>> Log:
>>>>> Test case for a delayed registration of WriteListener. Test is
> marked
>>> as
>>>>> @Ignore as it is failing with the current implementation.
>>>>>
>>>>> Although maybe not explicitly disallowed, I would assume you have
> to go
>>>> into non blocking mode right after startAsync in the container thread,
>>> and
>>>> it's not something asynchronous. Any other use probably doesn't make
> much
>>>> sense [even more after an upgrade]. The API lists the APIs that are
>>>> asynchronous and thread safe due to async (basically, AsyncContext)
> and
>>>> this is not one of them.
>>>> So I wouldn't fix this at the moment, especially if it adds
> complexity.
>>>
>>> The Processor for the "Request" that sets the WriteListener is in the
>>> waiting processors list.
>>> Can we introduce additional check if the Processor is in the waiting
>>> processors list then we can execute DISPATCH_EXECUTE.
>>>
>>>
>>> We just had a post on a user list who is trying to do exactly what I was
>> talking about: write operation going on, concurrently set
> setWriteListener.
>> As soon as async is allowed, people will do this, even if inadvertently
>> [the scenario you describe would work however, but there's no way to
>> guarantee it]. I remain convinced that setWriteListener is only allowed in
>> a container thread and not be concurrent with any IO operations, maybe we
>> should seek clarification from the EG though.
> 
> Yes I agree.
> I'll post a question in the EG mailing list for clarification.

If setWriteListener should be restricted to container threads, why would
it be a part of the public API?

-chris


Re: svn commit: r1765995 - /tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,

2016-10-27 13:53 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-10-23 8:22 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > Hi Remy,
> >
> > 2016-10-21 15:11 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> > >
> > > 2016-10-21 13:21 GMT+02:00 <vi...@apache.org>:
> > >
> > > > Author: violetagg
> > > > Date: Fri Oct 21 11:21:48 2016
> > > > New Revision: 1765995
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1765995&view=rev
> > > > Log:
> > > > Test case for a delayed registration of WriteListener. Test is
marked
> > as
> > > > @Ignore as it is failing with the current implementation.
> > > >
> > > > Although maybe not explicitly disallowed, I would assume you have
to go
> > > into non blocking mode right after startAsync in the container thread,
> > and
> > > it's not something asynchronous. Any other use probably doesn't make
much
> > > sense [even more after an upgrade]. The API lists the APIs that are
> > > asynchronous and thread safe due to async (basically, AsyncContext)
and
> > > this is not one of them.
> > > So I wouldn't fix this at the moment, especially if it adds
complexity.
> >
> > The Processor for the "Request" that sets the WriteListener is in the
> > waiting processors list.
> > Can we introduce additional check if the Processor is in the waiting
> > processors list then we can execute DISPATCH_EXECUTE.
> >
> >
> > We just had a post on a user list who is trying to do exactly what I was
> talking about: write operation going on, concurrently set
setWriteListener.
> As soon as async is allowed, people will do this, even if inadvertently
> [the scenario you describe would work however, but there's no way to
> guarantee it]. I remain convinced that setWriteListener is only allowed in
> a container thread and not be concurrent with any IO operations, maybe we
> should seek clarification from the EG though.

Yes I agree.
I'll post a question in the EG mailing list for clarification.

Regards,
Violeta

> Rémy

Re: svn commit: r1765995 - /tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Posted by Rémy Maucherat <re...@apache.org>.
2016-10-23 8:22 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:

> Hi Remy,
>
> 2016-10-21 15:11 GMT+03:00 Rémy Maucherat <re...@apache.org>:
> >
> > 2016-10-21 13:21 GMT+02:00 <vi...@apache.org>:
> >
> > > Author: violetagg
> > > Date: Fri Oct 21 11:21:48 2016
> > > New Revision: 1765995
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1765995&view=rev
> > > Log:
> > > Test case for a delayed registration of WriteListener. Test is marked
> as
> > > @Ignore as it is failing with the current implementation.
> > >
> > > Although maybe not explicitly disallowed, I would assume you have to go
> > into non blocking mode right after startAsync in the container thread,
> and
> > it's not something asynchronous. Any other use probably doesn't make much
> > sense [even more after an upgrade]. The API lists the APIs that are
> > asynchronous and thread safe due to async (basically, AsyncContext) and
> > this is not one of them.
> > So I wouldn't fix this at the moment, especially if it adds complexity.
>
> The Processor for the "Request" that sets the WriteListener is in the
> waiting processors list.
> Can we introduce additional check if the Processor is in the waiting
> processors list then we can execute DISPATCH_EXECUTE.
>
>
> We just had a post on a user list who is trying to do exactly what I was
talking about: write operation going on, concurrently set setWriteListener.
As soon as async is allowed, people will do this, even if inadvertently
[the scenario you describe would work however, but there's no way to
guarantee it]. I remain convinced that setWriteListener is only allowed in
a container thread and not be concurrent with any IO operations, maybe we
should seek clarification from the EG though.

Rémy

Re: svn commit: r1765995 - /tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Posted by Violeta Georgieva <mi...@gmail.com>.
Hi Remy,

2016-10-21 15:11 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-10-21 13:21 GMT+02:00 <vi...@apache.org>:
>
> > Author: violetagg
> > Date: Fri Oct 21 11:21:48 2016
> > New Revision: 1765995
> >
> > URL: http://svn.apache.org/viewvc?rev=1765995&view=rev
> > Log:
> > Test case for a delayed registration of WriteListener. Test is marked as
> > @Ignore as it is failing with the current implementation.
> >
> > Although maybe not explicitly disallowed, I would assume you have to go
> into non blocking mode right after startAsync in the container thread, and
> it's not something asynchronous. Any other use probably doesn't make much
> sense [even more after an upgrade]. The API lists the APIs that are
> asynchronous and thread safe due to async (basically, AsyncContext) and
> this is not one of them.
> So I wouldn't fix this at the moment, especially if it adds complexity.

The Processor for the "Request" that sets the WriteListener is in the
waiting processors list.
Can we introduce additional check if the Processor is in the waiting
processors list then we can execute DISPATCH_EXECUTE.

Regards,
Violeta

> Rémy

Re: svn commit: r1765995 - /tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Posted by Rémy Maucherat <re...@apache.org>.
2016-10-21 13:21 GMT+02:00 <vi...@apache.org>:

> Author: violetagg
> Date: Fri Oct 21 11:21:48 2016
> New Revision: 1765995
>
> URL: http://svn.apache.org/viewvc?rev=1765995&view=rev
> Log:
> Test case for a delayed registration of WriteListener. Test is marked as
> @Ignore as it is failing with the current implementation.
>
> Although maybe not explicitly disallowed, I would assume you have to go
into non blocking mode right after startAsync in the container thread, and
it's not something asynchronous. Any other use probably doesn't make much
sense [even more after an upgrade]. The API lists the APIs that are
asynchronous and thread safe due to async (basically, AsyncContext) and
this is not one of them.
So I wouldn't fix this at the moment, especially if it adds complexity.

Rémy