On 17/06/2014 00:50, Konstantin Kolinko wrote:
> 2014-06-14 0:21 GMT+04:00  <ma...@apache.org>:
>> Author: markt
>> Date: Fri Jun 13 20:21:32 2014
>> New Revision: 1602510

<snip/>

> I have several questions.
> 
> 1) The AbstractProtocol$AbstractConnectionHandler.process() mentioned
> above is from Tomcat 8. The code for handling SocketStatus.CLOSE_NOW
> there is missing in backport to Tomcat 7.
> 
> In Tomcat 7 the SocketStatus.CLOSE_NOW enum value is used only in 1
> place, where in Tomcat 8 it is used in 2 places..

It looks like I messed up the merge of that part of the back-port. Fixed
in r1603152.

> 2) The submission of SocketProcessor to the Executor may end with
> RejectedExecutionException.
> - in NioEndpoint#processSocket(NioChannel, SocketStatus, boolean)
> 
> In that case it just logs a warning and does nothing else.
> 
> How likely is this rejected execution?

Very unlikely.

> My understanding/answer is that the rejection is unlikely.
> The executor here is o.a.c.core.StandardThreadExecutor. Its execute()
> method is unlikely to reject tasks. There is a queue that accepts
> tasks. Tasks will be rejected if either
> a) Executor is not running.
> b) Queue is full. The maximum queue length is Integer.MAX_VALUE by
> default. Though its size is configurable via
> StandardThreadExecutor.setMaxQueueSize().
> 
> Will it result in failing to decrement the connection counter?
> Will it result in failing to obtain and to recycle the processor
> associated with the socket?
> 
> I suspect that per this BZ 56518 it will fail to decrement the counter.

The only time I think this might occur is during connector stop() in
which case the socket will be closed and the connection counter reset
anyway.

> 3) In case if there are several executor threads that process the
> socket at the same time, what happens?
> 
> My understanding/answer is that "synchronized (socket)" in
> NioEndpoint$SocketProcessor.run() handles that.

Correct. You don't want multiple threads trying to read from the socket
at the same time. Neither do you want multiple threads trying to writ to
the socket at the same time. One thread reading and one thread writing
is OK.

> In this case I see that NioEndpoint$SocketProcessor.doRun() does
> "nioChannels.offer(socket)"
> That is it offers the "socket" for reuse.
> 
> Thus "synchronized (socket)" is broken, as the "socket" can be
> recycled and reused by another thread while the second one waits to
> obtain the monitor on the "socket".

In theory yes. In practise, I don't believe that this will happen.

The syncs were added as part of the Servlet 3.0 async processing
implementation. The case they were trying to deal with was:
a) app on container thread starts async processing
b) app passes control to app thread
c) container thread is returned to container for reuse
   (update state, does a little clean up before it is reused)
d) app thread completes the async processing
e) control returns to container thread to complete the request

There was a chance that e) could complete before c) and that caused all
sorts of confusion. The syncs ensure c) completes before e) starts.

For the problem you describe to occur a fatal I/O error would have to
occur during c) after e) had reached the sync. Since there should be no
I/O during c) I don't think this can happen.

If you have a scenario where you think this is an issue, please share it.

> 4) There are no tests for this new feature.

I have been using the test WAR provided by the OP. I haven't had a
chance to convert it to a unit test. Obviously, no objections from me if
you want to do that.

Mark


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

Reply via email to