quanah / net-ldapapi

The Net::LDAPapi Perl Module uses the OpenLDAP and Mozilla C api's to directly access and manipulate an LDAP v2 or LDAP v3 server.
8 stars 6 forks source link

Memory leak in Net::LDApapi listen_for_changes #51

Closed macrotex closed 5 years ago

macrotex commented 5 years ago

We are seeing a slow but steady memory leak when using the listen_for_changes function from Net::LDAPapi. Here is code that I run that exhibits the problem:

sub listen_for_changes1 {

    my $this_base = 'cn=accesslog';
    my $this_filter = 'objectclass=*';
    my $this_cookie = '/var/run/ldap-sync-suprivilegegroup.cookie';

    if (!(-e $this_cookie)) {
        open(my $f, q{>}, $this_cookie);
        close($f);
    }

    my $msgid = $LDAP->listen_for_changes(-basedn  => $this_base,
                                          -scope   => LDAP_SCOPE_SUBTREE,
                                          -filter  => $this_filter,
                                          -cookie  => $this_cookie);
    while (1) {
        while (my @entries = $LDAP->next_changed_entries($msgid, 0, -1)) {
        }
    }
}

If I run the above code and then continually make changes to the data, the memory use of the perl script slowly (but surely) increases until after about a week it is using up most of the memory on the server.

Here are the relevant Debian packages installed on this server:

ii  ldap-utils                         2.4.47+dfsg-1~bpo9+1su1
ii  libldap-2.4-2:amd64                2.4.47+dfsg-1~bpo9+1su1
ii  libldap-common                     2.4.47+dfsg-1~bpo9+1su1
ii  libnet-ldap-perl                   1:0.6500+dfsg-1
ii  libnet-ldapapi-perl                3.0.5-1~bpo9+1 
phillipod commented 5 years ago

Hrm - I don't currently have a dev environment setup so I'm currently unable to replicate myself, but could you please tell me if that code snippet, assuming a valid $LDAP, is an actual test case that leaks as-is, without any use of the returned entries within the loop?

When you say it's using up most of the memory after a week, how much memory is that in GB, and could you please give a rough idea of the volume of changes per day?

I recall that the Net::LDAPapi listen_for_changes/next_changed_entries pretty much is just a Perl wrapper that takes the hassle out of handling the syncrepl control ASN and creating/parsing the controls to be fed into ldap_search etc. So unless there is evidence of widespread memory leakage on normal ldap_search, it's likely any leak will be in the Perl implementation for cookies or BER encoding/decoding.

I'll have a bit of a look and see if I can spot anything obvious

quanah commented 5 years ago

It appears the leak was introduced in https://github.com/quanah/net-ldapapi/commit/d18a1f04afa0939534ce93caa12d0fe606138206#diff-74642b2d4269686ea9d4c77906c39dab.

In LDAPapi.pm, If line 1558 and 1560 are reverted to use $asn->encode instead of $syncRequestValue->encode the memory leak appears to stop. This, however causes the module to fail when starting up if the sync cookie file is missing or empty.

On line 1607, the call to $asn->encode doesn't return a value, and the module dies when $self->create_control is called on 1611 because the $syncRequestBerval is empty.