|Summary:||Selector is not closed.|
|Product:||Tomcat 6||Reporter:||Hao Zhong <ackel>|
|Component:||Catalina||Assignee:||Tomcat Developers Mailing List <dev>|
Description Hao Zhong 2008-08-11 22:45:15 UTC
In the class ParallelNioSender , a Selector (selector) is declared as a field. In line 60, it is opened (selector = Selector.open()). However, it is not closed in any situations. As shown by its Javadoc , it has a close() method, and this method should be called to close the selector. http://www.google.com/codesearch?hl=en&q=lang:java+open+java.nio.channels.Selector+show:hsMlF-2s6UU:ZSXpd2XI4bI:dK-_LkuTzG4&sa=N&ct=rx&cd=4&cs_p=http://archive.apache.org/dist/tomcat/tomcat-6/v6.0.0/src/apache-tomcat-6.0.0-src.tar.gz&cs_f=apache-tomcat-6.0.0-src/java/org/apache/catalina/tribes/transport/nio/ParallelNioSender.java&cs_p=http://archive.apache.org/dist/tomcat/tomcat-6/v6.0.0/src/apache-tomcat-6.0.0-src.tar.gz&cs_f=apache-tomcat-6.0.0-src/java/org/apache/catalina/tribes/transport/nio/ParallelNioSender.java http://java.sun.com/j2se/1.5.0/docs/api/java/nio/channels/Selector.html
Comment 1 Mark Thomas 2008-08-12 07:01:06 UTC
Have you observed a memory leak associated with these objects? If so, in what circumstances?
Comment 2 Hao Zhong 2008-08-13 18:17:39 UTC
(In reply to comment #1) > Have you observed a memory leak associated with these objects? If so, in what > circumstances? Actually, I am a PhD student in Computer Science and I am conducting an experiment on automatically detecting bugs. As my approach detected the bug statically, I cannot provide in what circumstances it will cause memory leak. Still, I will be quite happy if my tool really finds some useful results for you.
Comment 3 Mark Thomas 2008-08-14 01:29:21 UTC
Given how the selector is used, I can't see this causing a problem. It is my experience that automated tools tend to produce a lot of false positives. Any automated analysis of the Tomcat code base is welcome but there is a risk that a large amount of effort is expended investigating potential issues that turn out not to be bugs. The best way to avoid this is to filter issues before reporting them. Ideally, a bug report should contain a test case that demonstrates the issue, eg memory leak. If that isn't possible, then an explanation of how the issue may occur. As I stated above, I can't see how this could cause an issue so I am closing it as invalid. If you believe this to be incorrect, please re-open this report with an explanation of how not closing the selector leads to problems.
Comment 4 Hao Zhong 2008-08-14 18:48:32 UTC
Please take a look at a confirmed bug in JDK 1.5 . This bug is caused by not explicitly closing a selector. I copy some descriptions from the bug report as follows. "However, when SocketAdaptor is done with the temporarily created selector, it never explicitly calls selector.close(), which causes the Pipe-related sockets to be orphaned and left in an ESTABLISHED state until process death. Over time, this exhausts the number of available sockets and degrades both application and operating system performance. Pipe-related sockets are only cleaned up when the owning process is killed." I agree that automatic tools can produce some false positives. As you suggested, I do not report all the violations but report those violations that make sense in my opinion. I am sorry that my tool cannot provide in what situation a bug will cause problem as my tool relies on static analysis. Still, I believe that you should take the report into careful considerations because as you can see, not closing a selector has already cause some problems in JDK 1.5 and they do take efforts to fix the problems.  http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5083450
Comment 5 Mark Thomas 2008-09-02 14:46:37 UTC
I still can't see this causing an issue in any real world deployment. You would have to go out of your way to create the circumstances where this bug would occur. I came close to closing this as INVALID again for this reason. However, there is - technically - a bug here so it has been fixed in trunk and proposed for 6.0.x.
Comment 6 Mark Thomas 2008-10-09 05:37:12 UTC
The fix has been applied to 6.0.x and will be included in 6.0.19 onwards.