You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "Guangtai Liang (Created) (JIRA)" <se...@james.apache.org> on 2012/02/23 03:23:48 UTC

[jira] [Created] (JAMES-1384) An incomplete fix for the resource leak bugs in SMTPHandler.java

An incomplete fix for the resource leak bugs in SMTPHandler.java
----------------------------------------------------------------

                 Key: JAMES-1384
                 URL: https://issues.apache.org/jira/browse/JAMES-1384
             Project: JAMES Server
          Issue Type: Bug
          Components: SMTPServer
    Affects Versions: 2.3.2, 2.3.1, 2.3.0
            Reporter: Guangtai Liang
            Priority: Critical


The fix revision 108172 was aimed to remove resource leak bugs on the Socket object "this.socket" (created in line 275), the BufferedInputStream object "in" (line 276), the BufferedReader object "inReader" (line 280), the InternetPrintWriter object "out" (line 312) in the method "handleConnection"of the file "/james/server/trunk/src/java/org/apache/james/smtpserver/SMTPHandler.java.java (now moved to /james/server/branches/v2.3/src/java/org/apache/james/smtpserver/SMTPHandler.java)" , but it is incomplete.


There are some problems: 
1. Only "socket" is closed explicitly, other resource objects "in", "inReader", "out" are not closed .
2. when "inReader" isn't created successfully but the temp InputStreamReader object is created successfully (at line 280), the temp InputStreamReader object will be leaked. 
3. when the statements at lines 299-307 throw exceptions, the "socket", "in", "inReader" will be leaked. 
4. when "out" isn't created successfully but the temp BufferedWriter object is created successfully (at line 312), the temp BufferedWriter object will be leaked. 
5. when the temp BufferedWriter object isn't created successfully but the temp OutputStreamWriter object is created successfully (at line 312), the temp OutputStreamWriter object will be leaked. 

The best way to close such resource objects is putting such close operations for all resource 
objects in the finaly block of a try-catch-finally structure and then putting all other code in a try block.

The problem still exists in the head revision. The buggy code is copied as bellows:

public void handleConnection(Socket connection) throws IOException {

        try {
275            this.socket = connection;
276            in = new BufferedInputStream(socket.getInputStream(), 1024);
            // An ASCII encoding can be used because all transmissions other
            // that those in the DATA command are guaranteed
            // to be ASCII
280            inReader = new BufferedReader(new InputStreamReader(in, "ASCII"), 512);
            remoteIP = socket.getInetAddress().getHostAddress();
            remoteHost = socket.getInetAddress().getHostName();
            smtpID = random.nextInt(1024) + "";
            resetState();
        } catch (Exception e) {
                     ......
        }

 299       if (getLogger().isInfoEnabled()) {
                     ......
 307           getLogger().info(infoBuffer.toString());
        }

        try {

312            out = new InternetPrintWriter(new BufferedWriter(new OutputStreamWriter

(socket.getOutputStream()), 1024), false);

            // Initially greet the connector
            // Format is:  Sat, 24 Jan 1998 13:16:09 -0500
          ......
        } catch (SocketException se) {
                     ......
        } catch ( InterruptedIOException iioe ) {
          ......
        } catch ( IOException ioe ) {
          ......
        } catch (Exception e) {
          ......
        } finally {
 372           resetHandler();
        }
    }


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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


[jira] [Commented] (JAMES-1384) An incomplete fix for the resource leak bugs in SMTPHandler.java

Posted by "Eric Charles (Commented) (JIRA)" <se...@james.apache.org>.
    [ https://issues.apache.org/jira/browse/JAMES-1384?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13214493#comment-13214493 ] 

Eric Charles commented on JAMES-1384:
-------------------------------------

Hi there, Thx for the patch!
We don't plan a maintenance release for james 2.3 soon :/, so your best bet is to build your own with the patch applied.
Eric

                
> An incomplete fix for the resource leak bugs in SMTPHandler.java
> ----------------------------------------------------------------
>
>                 Key: JAMES-1384
>                 URL: https://issues.apache.org/jira/browse/JAMES-1384
>             Project: JAMES Server
>          Issue Type: Bug
>          Components: SMTPServer
>    Affects Versions: 2.3.0, 2.3.1, 2.3.2
>            Reporter: Guangtai Liang
>            Priority: Critical
>   Original Estimate: 10m
>  Remaining Estimate: 10m
>
> The fix revision 108172 was aimed to remove resource leak bugs on the Socket object "this.socket" (created in line 275), the BufferedInputStream object "in" (line 276), the BufferedReader object "inReader" (line 280), the InternetPrintWriter object "out" (line 312) in the method "handleConnection"of the file "/james/server/trunk/src/java/org/apache/james/smtpserver/SMTPHandler.java.java (now moved to /james/server/branches/v2.3/src/java/org/apache/james/smtpserver/SMTPHandler.java)" , but it is incomplete.
> There are some problems: 
> 1. Only "socket" is closed explicitly, other resource objects "in", "inReader", "out" are not closed .
> 2. when "inReader" isn't created successfully but the temp InputStreamReader object is created successfully (at line 280), the temp InputStreamReader object will be leaked. 
> 3. when the statements at lines 299-307 throw exceptions, the "socket", "in", "inReader" will be leaked. 
> 4. when "out" isn't created successfully but the temp BufferedWriter object is created successfully (at line 312), the temp BufferedWriter object will be leaked. 
> 5. when the temp BufferedWriter object isn't created successfully but the temp OutputStreamWriter object is created successfully (at line 312), the temp OutputStreamWriter object will be leaked. 
> The best way to close such resource objects is putting such close operations for all resource 
> objects in the finaly block of a try-catch-finally structure and then putting all other code in a try block.
> The problem still exists in the head revision. The buggy code is copied as bellows:
> public void handleConnection(Socket connection) throws IOException {
>         try {
> 275            this.socket = connection;
> 276            in = new BufferedInputStream(socket.getInputStream(), 1024);
>             // An ASCII encoding can be used because all transmissions other
>             // that those in the DATA command are guaranteed
>             // to be ASCII
> 280            inReader = new BufferedReader(new InputStreamReader(in, "ASCII"), 512);
>             remoteIP = socket.getInetAddress().getHostAddress();
>             remoteHost = socket.getInetAddress().getHostName();
>             smtpID = random.nextInt(1024) + "";
>             resetState();
>         } catch (Exception e) {
>                      ......
>         }
>  299       if (getLogger().isInfoEnabled()) {
>                      ......
>  307           getLogger().info(infoBuffer.toString());
>         }
>         try {
> 312            out = new InternetPrintWriter(new BufferedWriter(new OutputStreamWriter
> (socket.getOutputStream()), 1024), false);
>             // Initially greet the connector
>             // Format is:  Sat, 24 Jan 1998 13:16:09 -0500
>           ......
>         } catch (SocketException se) {
>                      ......
>         } catch ( InterruptedIOException iioe ) {
>           ......
>         } catch ( IOException ioe ) {
>           ......
>         } catch (Exception e) {
>           ......
>         } finally {
>  372           resetHandler();
>         }
>     }

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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