You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2015/05/22 08:21:27 UTC

[Bug 57943] New: NioEndpoint, one poller thread died

https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

            Bug ID: 57943
           Summary: NioEndpoint, one poller  thread died
           Product: Tomcat 7
           Version: 7.0.54
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: critical
          Priority: P2
         Component: Connectors
          Assignee: dev@tomcat.apache.org
          Reporter: sunqi800@163.com

when i restart tomcat,the tomcat can not process request soon,i find an error
log

Exception in thread "http-nio-7001-ClientPoller-1"
java.util.ConcurrentModificationException
        at java.util.HashMap$HashIterator.nextEntry(HashMap.java:793)
        at java.util.HashMap$KeyIterator.next(HashMap.java:828)
        at
java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1010)
        at
org.apache.tomcat.util.net.NioEndpoint$Poller.timeout(NioEndpoint.java:1421)
        at
org.apache.tomcat.util.net.NioEndpoint$Poller.run(NioEndpoint.java:1215)
        at java.lang.Thread.run(Thread.java:662)

tomcat poller thread is not catch this exception,so this thread is died.
connection is accept but has no poller thread to process.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 57943] NioEndpoint, one poller thread died

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

Ian Luo <ia...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ian.luo@gmail.com

--- Comment #4 from Ian Luo <ia...@gmail.com> ---
Created attachment 32880
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=32880&action=edit
analysis for bug57943

it's in markdown format

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 57943] NioEndpoint, one poller thread died

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

Konstantin Kolinko <kn...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Konstantin Kolinko <kn...@gmail.com> ---
Fixed in Tomcat 6 in r1710473 and will be in 6.0.45 onwards.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 57943] NioEndpoint, one poller thread died

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
I have added the work-around to trunk (for 9.0.x), 8.0.x (for 8.0.24 onwards)
and 7.0.x (for 7.0.63 onwards).

I suspect a JVM bug so I am resolving this as FIXED. If we discover it is a
Tomcat bug we can always re-open.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 57943] NioEndpoint, one poller thread died

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

Christopher Schultz <ch...@christopherschultz.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32880|text/markdown               |text/plain
          mime type|                            |

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


Re: [Bug 57943] NioEndpoint, one poller thread died

Posted by Ian Luo <ia...@gmail.com>.
Hi Chris, Sure, just done.

Thanks,
-Ian.

On Thu, Jul 2, 2015 at 9:55 PM, Christopher Schultz <
chris@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, <bu...@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-unsubscribe@tomcat.apache.org
> >> For additional commands, e-mail: dev-help@tomcat.apache.org
> >>
> >>
> >
>
>

Re: [Bug 57943] NioEndpoint, one poller thread died

Posted by Christopher Schultz <ch...@christopherschultz.net>.
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, <bu...@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-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
> 


Re: [Bug 57943] NioEndpoint, one poller thread died

Posted by Ian Luo <ia...@gmail.com>.
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, <bu...@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

[Bug 57943] NioEndpoint, one poller thread died

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


[Bug 57943] NioEndpoint, one poller thread died

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

sunqi <su...@163.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|RESOLVED                    |REOPENED

--- Comment #3 from sunqi <su...@163.com> ---
(In reply to Mark Thomas from comment #2)
> I have added the work-around to trunk (for 9.0.x), 8.0.x (for 8.0.24
> onwards) and 7.0.x (for 7.0.63 onwards).
> 
> I suspect a JVM bug so I am resolving this as FIXED. If we discover it is a
> Tomcat bug we can always re-open.

we know the reason why poller thread died,as this bug
https://bz.apache.org/bugzilla/show_bug.cgi?id=57340

we use async servlet, many request is async timeout,at the same time,async
servlet calls complete or dispatch


nioChannels cache contain 2 references to the same object,and two request will
use same NioChannel,and poller object will replace by other request

wo also find a new place put the same NioChannel and AttachmentKey into caches. 

     } else if (handshake == -1 ) {
                    if (key != null) {
                        socket.getPoller().cancelledKey(key,
SocketStatus.DISCONNECT);
                    }
                    if (running && !paused) {
                        nioChannels.push(socket);
                    }
                    socket = null;
                    if (running && !paused) {
                        keyCache.push(ka);
                    }
                    ka = null;
}

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 57943] NioEndpoint, one poller thread died

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Product|Tomcat 7                    |Tomcat 6
          Component|Connectors                  |Connectors
            Version|7.0.54                      |6.0.44
   Target Milestone|---                         |default

--- Comment #6 from Mark Thomas <ma...@apache.org> ---
Many thanks for the details analysis.

I have applied a patch that should fix this to trunk (9.0.x) and back-ported it
to 8.0.x (for 8.0.25 onwards) and 7.0.x (7.0.64 onwards). I have also proposed
is for 6.0.x.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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


[Bug 57943] NioEndpoint, one poller thread died

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=57943

--- Comment #5 from Christopher Schultz <ch...@christopherschultz.net> ---
Comment on attachment 32880
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=32880
analysis for bug57943

Ian's comment, not as an attachment, and therefore much more readable, is:

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:

```java
} 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:

```java
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:

```java
} 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:

```java
} 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:

```java
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.

```java
} 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;
```

-- 
You are receiving this mail because:
You are the assignee for the bug.

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