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

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        1d 1h 31m 1 Jean-Baptiste Onofré 08/Nov/11 15:46
        In Progress In Progress Resolved Resolved
        3h 30m 1 Jean-Baptiste Onofré 08/Nov/11 19:16
        Resolved Resolved Reopened Reopened
        16d 17h 25m 1 metatech 25/Nov/11 12:41
        Reopened Reopened Resolved Resolved
        3h 42m 1 Jean-Baptiste Onofré 25/Nov/11 16:24
        Resolved Resolved Closed Closed
        62d 9h 35m 1 Jamie goodyear 27/Jan/12 01:59
        Jamie goodyear made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Jean-Baptiste Onofré made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        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.
        Hide
        Jean-Baptiste Onofré added a comment -

        Fixed on trunk: revision 1206232.

        Show
        Jean-Baptiste Onofré added a comment - Fixed on trunk: revision 1206232.
        Jean-Baptiste Onofré made changes -
        Fix Version/s 2.2.5 [ 12317857 ]
        Fix Version/s 3.0.0 [ 12316040 ]
        metatech made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        metatech added a comment -

        One missing close (patch in comment)

        Show
        metatech added a comment - One missing close (patch in comment)
        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.
        Jean-Baptiste Onofré made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        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
        Jean-Baptiste Onofré added a comment -

        Fixed on trunk: revision 1199373.

        Show
        Jean-Baptiste Onofré added a comment - Fixed on trunk: revision 1199373.
        Jean-Baptiste Onofré made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Jean-Baptiste Onofré made changes -
        Assignee Jean-Baptiste Onofré [ jbonofre ]
        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
        Glen Mazza made changes -
        Field Original Value New Value
        Attachment LDAPLoginModule.patch [ 12502882 ]
        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).
        metatech created issue -

          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