Hi Chris, Sure, just done.

Thanks,
-Ian.

On Thu, Jul 2, 2015 at 9:55 PM, Christopher Schultz <
ch...@christopherschultz.net> wrote:

> Ian,
>
> This is a nice follow-up. Could you attach it directly to the bug report
> in Bugzilla, so the commentary is all in one place?
>
> https://bz.apache.org/bugzilla/show_bug.cgi?id=57943
>
> Thanks,
> -chris
>
> On 7/2/15 6:29 AM, Ian Luo wrote:
> > Hi Mark,
> >
> > I am the colleague of Sun Qi who reported bug57943
> > <https://bz.apache.org/bugzilla/show_bug.cgi?id=57943> one month ago. I
> > believe we managed to find out why ConcurrentModificationException
> happens
> > in our scenario.
> >
> > Here’s the scenario, our application uses async servlet to write the
> > response back to the client. The operation we put in async servlet which
> > run in another thread is quite time-consuming. When the incoming traffic
> is
> > heavy, which usually happens when the instances of tomcat server reboots,
> > we hit this issue very easily.
> >
> > When the ClientPoller thread run into timeout method (in NioEndpoint) and
> > execute the following code:
> >
> > } else if (!ka.isAsync() || ka.getTimeout() > 0) {
> >     // Async requests with a timeout of 0 or less never timeout
> >     long delta = now - ka.getLastAccess();
> >     long timeout =
> >
> (ka.getTimeout()==-1)?((long)socketProperties.getSoTimeout()):(ka.getTimeout());
> >     boolean isTimedout = delta > timeout;
> >     if (isTimedout) {
> >         // Prevent subsequent timeouts if the timeout event takes a
> > while to process
> >         ka.access(Long.MAX_VALUE);
> >         processSocket(ka.getChannel(), SocketStatus.TIMEOUT, true, 6);
> >     }}
> >
> > Eventually it calls processSocket(ka.getChannel()) and puts back the
> > NioChannel instance to the cache nioChannels for the late re-use:
> >
> > if (ka!=null) ka.setComet(false);
> > socket.getPoller().cancelledKey(key, SocketStatus.ERROR, false);if
> > (running && !paused) {
> >     nioChannels.offer(socket);}
> >
> > Unfortunately in our scenarios, when QPS is greater than 1000req/sec,
> async
> > servlet threads have very large chance to start async-complete and begin
> to
> > call processSocket (in Http11NioProcessor) at the same time:
> >
> > } else if (actionCode == ActionCode.ASYNC_COMPLETE) {
> >     if (asyncStateMachine.asyncComplete()) {
> >     ((NioEndpoint)endpoint).processSocket(socketWrapper.getSocket(),
> > SocketStatus.OPEN_READ, true, 10);}
> >
> > This leads to cache pollution in nioChannels since there’re chances to
> > offer the same object into the cache multiple times. When the Acceptor
> > thread polls the object and hands it over to the ClientPollers, the same
> > object could be pass into the different ClientPoller threads, then leads
> to
> > very serious problem.
> >
> > You may notice the code I show here is the code from tomcat 7.0.54, and
> > claim the similar issue has already been addressed, but this is not true.
> > We noticed the similar issue was reported on Bug57340
> > <https://bz.apache.org/bugzilla/show_bug.cgi?id=57340>, and the solution
> > has already been checked into the later release.
> >
> > But this solution is not a complete solution, in our scenario, there’s
> > chance to offer the duplicated object to the cache in else if clause
> where
> > the solution for Bug57340 doesn’t cover:
> >
> > } else if (handshake == -1 ) {
> >     if (key != null) {
> >         socket.getPoller().cancelledKey(key, SocketStatus.DISCONNECT,
> false);
> >     }
> >     nioChannels.offer(socket);
> >     socket = null;
> >     if ( ka!=null ) keyCache.offer(ka);
> >     ka = null;}
> >
> > This is easily to understand. When handshake equals -1, one possibility
> is
> > the key passed into the doRun method is null, and when key is null, it
> > means some other thread has already finished processing on the same
> socket:
> >
> > public void run() {
> >     SelectionKey key = socket.getIOChannel().keyFor(
> >         socket.getPoller().getSelector());
> >
> > We propose the change below in order to address this issue throughly. In
> > this case, I think we can simply drop the object instead of offering it
> > since it looks there’s no other ideal way to not use lock.
> >
> > } else if (handshake == -1 ) {
> >     if (key != null) {
> >         if (socket.getPoller().cancelledKey(key,
> > SocketStatus.DISCONNECT, false) != null) {
> >             nioChannels.offer(socket);
> >             if (ka != null) keyCache.offer(ka);
> >         }
> >     }
> >     socket = null;
> >     ka = null;
> >
> > Best Regards,
> > -Ian.
> >
> > On Fri, May 22, 2015 at 9:29 PM, <bugzi...@apache.org> wrote:
> >
> >> https://bz.apache.org/bugzilla/show_bug.cgi?id=57943
> >>
> >> --- Comment #1 from Mark Thomas <ma...@apache.org> ---
> >> This is the third report we have received about this issue.
> >>
> >> The other two are:
> >> http://markmail.org/message/xgxblpk4v4ykgi5y
> >> http://markmail.org/message/eypk42i6gdpztkpy
> >>
> >> I have looked through the NIO Poller code again and I still can't see
> how
> >> this
> >> can be happening. I've also dug into the JRE code and I can't see any
> >> issues
> >> there either (although I'll not this has been reported on a variety of
> >> JVMs and
> >> I haven't checked the specific ones mentions).
> >>
> >> I think the best thing to do at this point is catch the error, log a
> >> warning
> >> and maybe ask folks to report it if they can repeat it.
> >>
> >> --
> >> You are receiving this mail because:
> >> You are the assignee for the bug.
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> >> For additional commands, e-mail: dev-h...@tomcat.apache.org
> >>
> >>
> >
>
>

Reply via email to