Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why compiler warns about resource leak?

I cannot understand why compiler warns me about a resource leak (Resource leak: 'conn' is not closed at this location) in the following code:

    Connection conn = null;
    try {
        conn = DatabaseConnectionPool.getConnectionFromPool();
        // Some code
        try {
            // Other code
        } catch (final SomeException e) {
            // More code
            throw e; // Resource leak: 'conn' is not closed at this location
        }

    } catch (final SQLException | OtherExceptions e) {
        // Some more code
    } finally {
        try {
            // Another bunch of code
        } finally {
            DatabaseConnectionPool.freeConnection(conn);
        }
    }

Note that if I write it like this

    Connection conn = null;
    try {
        conn = DatabaseConnectionPool.getConnectionFromPool();
        // Some code
        try {
            // Other code
        } catch (final SomeException e) {
            // More code
            throw e;
        } finally {
            DatabaseConnectionPool.freeConnection(conn);
        }

    } catch (final SQLException | OtherExceptions e) {
        // Some more code
    } finally {
        // Another bunch of code
    }

the warning is gone.

like image 692
m0skit0 Avatar asked Oct 20 '22 06:10

m0skit0


1 Answers

The compiler is quite dump. It probably cannot know that DatabaseConnectionPool.freeConnection(conn) will call close on the conn. I am not sure why the second example does not trigger this warning, but probably the feature is just not totally perfect and can yield false negatives. Basically, any resource should be closed by calling its close method directly at the place where the resource is acquired; this is the only way the compiler can figure out that you want to close it, it does not to inter-procedural analysis to check if a called function calls close.

With java7, you should consider using the try-with-resource statement; it is the encouraged way to handle any resources, i.e.:

try(Connection conn = ...){
   // Do something with conn

   // No close necessary here, done implicitly by the try statement
}

Your whole pattern of closing a connection by calling a method other than close seems flawed to me (it works, but I would strongly discourage its usage): Any resource should be able to close itself by calling close. If your resource requires DatabaseConnectionPool.freeConnection to be called to close it, you are violating java's resource contract. If you use the try-with-resource statement, you have no choice anyway: The statement will call close and not your method.

like image 108
gexicide Avatar answered Oct 24 '22 14:10

gexicide