LDAP connection leak in this code

Question

Could someone explain me why this piece of code randomly "leaks" LDAP connections? I can see the amount of established TCP/IP connections rising by time and at some stage this starts causing problems. I tried fiddling with the com.sun.jndi.ldap.connect environment properties (enabling pooling, disabling it and such) but it did not seem to help.

That means my lame piece of code has got bug(s). How should this done generally better and make sure I never "leak" LDAP connections?

import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
import javax.naming.directory.Attributes;
import javax.naming.directory.InitialDirContext;
import javax.naming.directory.SearchControls;
import javax.naming.directory.SearchResult;

public class LdapUtil {
    private String ldap_context = "ou=myunit,dc=com";
    protected static String ldap_server = "ldap://ldapserver:389/";
    private String ldap_prefix = "(&(uid=";
    private String ldap_postfix = ")(objectclass=inetOrgPerson))";
    private String[] ldap_attributes = {"uid","departmentNumber","cn","postOfficeBox","mail"};
    private Properties ldap_properties;
    private SearchControls ldap_searchcontrols;
        private static String ldap_principal = "uid=bind_account,cn=users,ou=myunit,dc=com";
        private static String ldap_credentials = "qwerty";

    private List<String> getUserAttributes(final String userId) {
        List<String> UserAttributes = new ArrayList<String>();
        InitialDirContext ctx = null;
        NamingEnumeration<SearchResult> resultsEnum = null;
        NamingEnumeration<String> atrEnum = null;

        // Connect the LDAP
        try {
        ctx = new InitialDirContext(this.ldap_properties);
        // Prepare the query string
        String query = this.ldap_prefix+ userId+ this.ldap_postfix;
        // Query!
        resultsEnum = ctx.search(this.ldap_context, query, this.ldap_searchcontrols);

        // Enumerate the results
        while (resultsEnum.hasMore()) {
            SearchResult sr = (SearchResult) resultsEnum.nextElement();

            // Get all the attributes for a hit
            Attributes atr = sr.getAttributes();
            // Enumerate the attributes
            atrEnum = atr.getIDs();

            while (atrEnum.hasMore()) {
                    String nextid = atrEnum.nextElement();
                    String nextattribute = atr.get(nextid).toString();
                    UserAttributes.add(nextattribute);
            }

        }
        } catch ( Exception eom ) {
            System.out.println("LDAP exception");
        } finally {
            try {
                if (atrEnum!=null)
                atrEnum.close();
                if (resultsEnum!=null)
                resultsEnum.close();
                if (ctx!=null)
                ctx.close();
            } catch (NamingException eo) {
                // nothing
            } catch (NullPointerException eo) {
                // Nothing
            }
        } 
        return UserAttributes;
    }


    /*
     * Parses the LDAP search results and searches for selected attribute.
     */
    private String getAttribute (final List<String> attributes,final String attribuutti) {
        String result = null;
        // Let's go through all attributes
        for (int i = 0; i < attributes.size(); i++) {
            String attribute = attributes.get(i).toString();
            // Look for match
            if (attribute.startsWith(attribuutti)) {
                // Return the value after the space
                int k = attribute.indexOf(" ");
                result = attribute.substring(k+1,attribute.length());
            }
        }
        return result;
    }


    public LdapUtil(String remoteuser) {

        // Pre-initialize LDAP connection related properties
        this.ldap_properties = new Properties();
        this.ldap_properties.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
        this.ldap_properties.put(Context.PROVIDER_URL, ldap_server) ;
        this.ldap_properties.put(Context.REFERRAL, "follow" );
        this.ldap_properties.put(Context.SECURITY_AUTHENTICATION, "simple");
        this.ldap_properties.put(Context.SECURITY_PRINCIPAL,ldap_principal);
        this.ldap_properties.put(Context.SECURITY_CREDENTIALS,ldap_credentials);
        this.ldap_properties.put("com.sun.jndi.ldap.read.timeout", "10000");
        this.ldap_properties.put("com.sun.jndi.ldap.connect.timeout", "10000");
        // Tried with both pooling and without - doesn't solve problems
        this.ldap_properties.put("com.sun.jndi.ldap.connect.pool", "false");

        // Pre-initialize LDAP search controls
        this.ldap_searchcontrols = new SearchControls();
        this.ldap_searchcontrols.setSearchScope(SearchControls.SUBTREE_SCOPE);
        this.ldap_searchcontrols.setReturningAttributes(this.ldap_attributes);

            // The List for attributes
        List<String> attributes = null;

        attributes = getUserAttributes(remoteuser);
    }

}

Answer 1

} finally {
    try {
        if (atrEnum!=null)
        atrEnum.close();
        if (resultsEnum!=null)
        resultsEnum.close();
        if (ctx!=null)
        ctx.close();
    } catch (NamingException eo) {
        // nothing
    } catch (NullPointerException eo) {
        // Nothing
    }
}

When atrEnum.close() fails with an exception, the resultsEnum and ctx will never be closed.

To fix this, just put all close() calls each in its own try/catch block.

} finally {
    if (atrEnum != null) try { 
        atrEnum.close(); 
    } catch (NamingException logOrIgnore) {}
    if (resultsEnum != null) try {
        resultsEnum.close();
    } catch (NamingException logOrIgnore) {}
    if (ctx != null) try {
        ctx.close();
    } catch (NamingException logOrIgnore) {}
}

Answer 2

One obvious issue is that your finally block is wrong. Ff close throws an exception the following close methods will not be called. Always write such code as:

final Res res = acquire();
try {
    use(res);
} finally {
    res.release();
}

Even if that means nesting try-finally.

(If you are using an old version of the Sun JRE there was an issue with LDAP (and Kerberos) resource leakage in an exception case. "SunSolve" page)

Answer 3

Thanks for the answers. I wrote this:

private void closeResources(final InitialDirContext ctx,final NamingEnumeration<SearchResult> resultsEnum,final NamingEnumeration<String> atrEnum) {
    try {
        atrEnum.close();
    } catch (Exception eom) {}

    try {
        resultsEnum.close();
    } catch (Exception eom) {}

    try {
        ctx.close();
    } catch (Exception eom) {}
}

and decided to call it from the finally block from previous. Slightly less messy to follow what is happening.