On Nov 26, 2013, at 2:57 PM, Mark Thomas <ma...@apache.org> wrote:

> On 26/11/2013 15:26, Daniel Mikusa wrote:
>> On Nov 26, 2013, at 10:13 AM, Daniel Mikusa <dmik...@gopivotal.com> wrote:
>> 
>>> On Nov 26, 2013, at 6:06 AM, Mark Thomas <ma...@apache.org> wrote:
>>> 
>>>> On 26/11/2013 10:46, Mark Thomas wrote:
>>>>> On 25/11/2013 17:39, Daniel Mikusa wrote:
>>>>>> On Nov 25, 2013, at 12:19 PM, "Caldarale, Charles R"
>>>>>> <chuck.caldar...@unisys.com> wrote:
>>>>>> 
>>>>>>>> From: Daniel Mikusa [mailto:dmik...@gopivotal.com] Subject:
>>>>>>>> Another Non-blocking IO Question
>>>>>>> 
>>>>>>>> Most of the time it works, but in one case I'm seeing requests
>>>>>>>> hang.
>>>>>>> 
>>>>>>> You know the ritual:
>>>>>>> 
>>>>>>> 1) Tomcat version? 2) JDK version? 3) Thread dump?
>>>>>> 
>>>>>> Doh… Tomcat trunk.  JDK 1.7.0_45.
>>>>>> 
>>>>>> Took some thread dumps, but I don't think that's going to help here.
>>>>>> There are no blocking threads or threads waiting on external
>>>>>> resources.  The issue is that the async context is not being closed
>>>>>> and, in my example this is not happening cause "onAllDataRead()" is
>>>>>> not being called.  Which back to my question, can I depend on
>>>>>> "onAllDataRead()" being called always or are there certain cases
>>>>>> where it won't get called?
>>>>> 
>>>>> You should be able to rely on onAllDataRead() being called.
>>>>> 
>>>>>>>> in this case "onWritePossible" (i.e. my WriteListener) is reading
>>>>>>>> the data.
>>>>>>> 
>>>>>>> That seems to be against the spirit of the spec, albeit not the
>>>>>>> letter.  Spec clarification might be needed here.
>>>>> 
>>>>> The nature of the Servlet 3.1 non-blocking API is such that if copying
>>>>> data from request to response that the entire body may be read and the
>>>>> entire response written without any calls to onDataAvailable() or
>>>>> onWritePossible(). (That case is handled.). Depending on network
>>>>> conditions you may see either or both of onDataAvailable() and
>>>>> onWritePossible().
>>>>> 
>>>>> The code doesn't currently handle it correctly if the request is
>>>>> finished during onWritePossible(). I need to add that but it is a little
>>>>> more complicated as there could be multiple onWritePossible() calls
>>>>> after the request has been finished.
>>>>> 
>>>>> Your test case would be very helpful.
>>>> 
>>>> Should be fixed in trunk but I still won;t say no to a test case.
>>> 
>>> I pulled down the latest trunk and my test is no longer hanging, so that 
>>> looks good.  It is throwing an NPE, but that could be my fault.  I haven't 
>>> investigated that yet.
>> 
>> NPE seems to be caused by my test case code.  I was throwing a 
>> RuntimeException in onAllDataRead() to show that it wasn't being called.  
>> Now that it's being called, the exception is being caught and triggering the 
>> NPE. 
>> 
>> It's generating an NPE because the exception is caught and in the catch 
>> block it's calling "onError(t)" for my write listener, however I completed 
>> the context prior to the exception being thrown so res.getWriteListener() is 
>> null.
>> 
>> Test case, completes the context and throws an exception...
>> 
>>              @Override
>>              public void onAllDataRead() throws IOException {
>>                  asyncContext.complete();
>>                  throw new RuntimeException("I never get called!");
>>              }
>> 
>> CoyoteAdapter line #370 is catching my exception, but res.getWriteListener() 
>> is null.
>> 
>>                    } catch (Throwable t) {
>>                        ExceptionUtils.handleThrowable(t);
>>                        res.getWriteListener().onError(t);  // <-- throws an 
>> NPE
>>                        throw t;
>>                   }
>> 
>> This is similar to what I reported in a different thread.
>> 
>>  http://markmail.org/message/mslnx7uggzftabkb
> 
> Should be fixed now.

Looks good!  Thanks for the quick fix!!

I'm seeing another issue with the same basic test though.  After I tested the 
previous changes and confirmed they worked, I increased the amount of data that 
the request was sending.

    int rc = postUrl(true, new StaticBytesStreamer(5 * 1048576, 131072, 0), 
"http://localhost:"; +
                            getPort() + "/", new ByteChunk(), resHeaders, null);

This uncovered a problem with my tests where I wasn't checking output.isReady() 
and I was able to fix that.  However after fixing that I'm getting another hang.

In this case, this is what seems to happen...    

1.) The test client sends 1441792 bytes of data
2.) onWritePossible is called, this in turn calls onDataAvailable and starts 
echoing data
2.) The test echoer is able to read and write back 696091 bytes successfully
3.) At this point there is more input to read, but the call to output.isReady() 
returns false and the initial call to onWritePossible finishes.
4.) I see a call to onDataAvailable from the container.  This checks the 
input.isReady() which is true, but output.isReady() is still false.  The call 
to onDataAvailable finishes.
5.) This is where the test hangs.

At this point, I would have expected onWritePossible to be called again by the 
container, since calls to output.isReady() returned false.  I haven't had much 
luck debugging this further.  

Here's the updated test case.

   https://gist.github.com/dmikusa-pivotal/7660005

Any thoughts?

Thanks

Dan



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

Reply via email to