ldap_result() doesn't honour passed timeout value.

[phillipod]

This code blocks indefinitely:

print "===== LISTEN_FOR_CHANGES / NEXT_CHANGED_ENTRIES\n";

my $listen_msg = $ld->listen_for_changes(
  -basedn => "dc=example,dc=com",
  -scope => LDAP_SCOPE_SUBTREE,
  -filter => "(objectClass=*)",
  -cookie => "/tmp/tmp.cookie", # has to exist first, even for first-run. There will be a pull request...
);

# First result should be Sync Info (LDAP_RES_INTERMEDIATE)
@entries = $ld->next_changed_entries($listen_msg, 0, 1);

# Second result should be empty if run after cookie has been updated,
# and therefore the timeout of "1" should be honoured
@entries = $ld->next_changed_entries($listen_msg, 0, 1);

LDAPapi.xs has this:

int
ldap_result(ld, msgid, all, timeout, result)
   ... 
   CODE:
    {
        struct timeval *tv_timeout = NULL, timeoutbuf;
        if (atof(timeout) > 0 && timeout && *timeout) { ... }
        RETVAL = ldap_result(ld, msgid, all, NULL, &result);
    }
   ...

The NULL parameter being passed is the timeout.

The result of this is that on OpenLDAP ldap_result() will use whatever has been set by ldap_set_options(), with the default being to wait forever (equivalent to timeout being set to -1), while Mozilla LDAP's ldap_result() will just wait forever.

In effect, synchronous operations work perfectly - but the asynchronous API does not - once ldap_result() is called, it's synchronous for the scope of of the all parameter unless a timeout has been set by ldap_set_options().

Note that fixing this has a knock-on impact to actually using next_changed_entries() in a timeout environment as next_changed_entries() does not support the case where a timeout has occurred.

I'll create a pull request to resolve both these things. Happily the test suite in #5 will already test the majority of the frequently consumed synchronous and asynchronous API .

[phillipod]

Hum. Note that I've tested the knock-on impact to using next_changed_entries() in a tree with ldap_result() fixed.

However, if the OpenLDAP C documentation for ldap_result() is correct and a NULL parameter being passed uses the value set with LDAP_OPT_TIMEOUT, then it should be possible to prove next_changed_entries() is broken on a non-modified build by setting a timeout like this.

However, this does not appear to work.

Investigating ldap_set_option(), it appears to do a fairly naive "just send this data to the function" - which works fine for pretty much most of the options supported by Mozilla LDAP. But OpenLDAP's support for 'struct timeval' options for LDAP_OPT_TIMEOUT and LDAP_OPT_NETWORK_TIMEOUT wouldn't work like this, and this is supported by ldap_set_option() returning -1.