Summary: | ConcurrentModificationException in NioReceiver on shutdown | ||
---|---|---|---|
Product: | Tomcat 7 | Reporter: | Casey Lucas <clucas> |
Component: | Cluster | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | 7.0.41 | ||
Target Milestone: | --- | ||
Hardware: | Sun | ||
OS: | Solaris | ||
Attachments: |
fix for bug
fix for bug |
Description
Casey Lucas
2012-11-01 13:32:28 UTC
Thanks for the report. I did see a code path that would result in two threads both trying to close the Selector. This, and some other threading issues, have been fixed in trunk and 7.0.x and will be included in 7.0.33 onwards. I can't be 100% certain that the problem I found is the one you were seeing but it looks very much like it. I verified that this problem still exists in 7.0.41. I tracked down and fixed the problem. Problem: During shutdown, NioReceiver.close() is called which ultimately calls NioReceiver.closeSelector(). selector.keys() is called and an iteration is performed - closing the existing channels and canceling the keys. Per the Selector javadocs, the set cannot be modified during iteration by another thread or else a ConcurrentModificationException will be thrown. The set is in fact being modified by another thread (NioReceiver thread) when existing connections are closed (see listen() and canceledKey(SelectionKey) methods). The problem is difficult to reproduce because of timing. I was able to reliably reproduce the problem but I had to add a sleep in the closeSelector() method and or set breakpoints during testing. Ultimately, I couldn't come up with a test case that didn't need a sleep in closeSelector() so I don't have a test case for the attached patch. The attached patch loops (up to 3 times) catching ConcurrentModificationException during close and sleeps for a brief period (100ms * attempt number) allowing the NioReceiver thread to finish it's loop. After the sleep, it attempts to close channels again. Created attachment 30489 [details]
fix for bug
Created attachment 30490 [details]
fix for bug
i inadvertently left in a printStackTrace on the original patch. use this one instead.
(In reply to Casey Lucas from comment #2) > Problem: > During shutdown, NioReceiver.close() There is no such method. I'm going to assume you mean stop(). Thanks for the analysis and patch. I went with a different approach as I generally prefer to tackle the root cause rather than handle the symptoms of a problem. Providing that the receiver thread stops in a timely manner, the patch prevents multiple threads accessing the key set. (In reply to Mark Thomas from comment #5) > (In reply to Casey Lucas from comment #2) > > Problem: > > During shutdown, NioReceiver.close() > > There is no such method. I'm going to assume you mean stop(). Yes, stop. Thanks for the fix. |