Karaf
  1. Karaf
  2. KARAF-985

LDAPLoginModule generates a large number of DirContext objects

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.4
    • Fix Version/s: 2.2.5, 3.0.0
    • Component/s: karaf-config
    • Labels:
      None
    • Environment:

      Karaf 2.2.5-snapshot

      Description

      In class org.apache.karaf.jaas.modules.ldap.LDAPLoginModule, "DirContext.close()" and "NamingEnumeration.close()" are not called in all cases.
      Although it is not mandatory, it would be better to call it in all cases (to be sure, in a "finally" block).
      http://download.oracle.com/javase/tutorial/jndi/ldap/close.html
      Otherwise, this leaves one "com.sun.jndi.ldap.Connection" thread running for each LDAP request. After several minutes, the thread disappears automatically.
      During a performance test with 10 concurrent users, this creates up to several hundreds of such threads.

        Activity

        Hide
        Glen Mazza added a comment -

        Possible solution for 3.0 branch (untested but builds fine and JUnit tests still work).

        Show
        Glen Mazza added a comment - Possible solution for 3.0 branch (untested but builds fine and JUnit tests still work).
        Hide
        Glen Mazza added a comment -

        The JavaDoc for NamingEnumeration.close()[1] says "If an enumeration proceeds to the end-that is, until hasMoreElements() or hasMore() returns false- resources will be freed up automatically and there is no need to explicitly call close()." For two out of the three cases with NamingEnumeration in this file, either of the two methods above will indeed return false, so close() shouldn't be needed then. I did add in a close() in the proposed patch for the third case.

        For DirContext(), I added in close() in the two of three scenarios where it wasn't already being called.

        [1] http://download.oracle.com/javase/6/docs/api/javax/naming/NamingEnumeration.html#close%28%29

        Show
        Glen Mazza added a comment - The JavaDoc for NamingEnumeration.close() [1] says "If an enumeration proceeds to the end- that is, until hasMoreElements() or hasMore() returns false - resources will be freed up automatically and there is no need to explicitly call close()." For two out of the three cases with NamingEnumeration in this file, either of the two methods above will indeed return false, so close() shouldn't be needed then. I did add in a close() in the proposed patch for the third case. For DirContext(), I added in close() in the two of three scenarios where it wasn't already being called. [1] http://download.oracle.com/javase/6/docs/api/javax/naming/NamingEnumeration.html#close%28%29
        Hide
        Jean-Baptiste Onofré added a comment -

        Fixed on trunk: revision 1199373.

        Show
        Jean-Baptiste Onofré added a comment - Fixed on trunk: revision 1199373.
        Hide
        Jean-Baptiste Onofré added a comment -

        Fixed on karaf-2.2.x: revision 1199391.

        Show
        Jean-Baptiste Onofré added a comment - Fixed on karaf-2.2.x: revision 1199391.
        Hide
        metatech added a comment -

        There is still one bug with the fix :
        In step 1, the "hasMore" is not called after the "next" is called, so the naming enumeration is not automatically closed.
        Here is the complete fix :
        [192] SearchResult result = (SearchResult) namingEnumeration.next();
        [193] userDN = (String) result.getName();
        + namingEnumeration.close();

        With the complete fix, maximum 10 LDAP connections are simultaneously in "ESTABLISHED" state (in netstat) with 10 concurrent users, instead of hundreds.

        Beware that there are still cases (although I cannot give an practical example) where a NamingException might be thrown by "next" or "hasMore", in which case the naming enumeration would not be read up to the end.

        Show
        metatech added a comment - There is still one bug with the fix : In step 1, the "hasMore" is not called after the "next" is called, so the naming enumeration is not automatically closed. Here is the complete fix : [192] SearchResult result = (SearchResult) namingEnumeration.next(); [193] userDN = (String) result.getName(); + namingEnumeration.close(); With the complete fix, maximum 10 LDAP connections are simultaneously in "ESTABLISHED" state (in netstat) with 10 concurrent users, instead of hundreds. Beware that there are still cases (although I cannot give an practical example) where a NamingException might be thrown by "next" or "hasMore", in which case the naming enumeration would not be read up to the end.
        Hide
        metatech added a comment -

        One missing close (patch in comment)

        Show
        metatech added a comment - One missing close (patch in comment)
        Hide
        Jean-Baptiste Onofré added a comment -

        Fixed on trunk: revision 1206232.

        Show
        Jean-Baptiste Onofré added a comment - Fixed on trunk: revision 1206232.
        Hide
        Jean-Baptiste Onofré added a comment -

        Fixed on karaf-2.2.x: revision 1206247.

        Show
        Jean-Baptiste Onofré added a comment - Fixed on karaf-2.2.x: revision 1206247.

          People

          • Assignee:
            Jean-Baptiste Onofré
            Reporter:
            metatech
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 2h
              2h
              Remaining:
              Remaining Estimate - 2h
              2h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development