Bug #85941 for perl-ldap: Net::LDAP 'process' method blocking in Async mode

Fri Jun 07 02:53:27 2013 chris [...] wolfhugel.eu - Ticket created

Subject:	Net::LDAP 'process' method blocking in Async mode
Date:	Fri, 07 Jun 2013 07:53:15 +0100
To:	bug-perl-ldap [...] rt.cpan.org
From:	Christophe Wolfhugel <chris [...] wolfhugel.eu>

I do think following is a bug in 0.55 (and earlier), as this code would block on the first loop: sub process { my $ldap = shift; my $what = shift; my $sock = $ldap->socket or return LDAP_SERVER_DOWN; my $sel = IO::Select->new($sock); my $ready; for ($ready = 1 ; $ready ; $ready = $sel->can_read(0) || (ref($sock) eq 'IO::Socket::SSL' && $sock->pending())) { my $pdu; asn_read($sock, $pdu) or return _drop_conn($ldap, LDAP_OPERATIONS_ERROR, 'Communications Error'); When calling the process method, asn_read will always be called on the first loop, eventually blocking the process even if no messages are available. I would believe that the loop could/should be tried as something similar to: while (defined $sel->can_read(0) || (ref($sock) eq 'IO::Socket::SSL' && $sock->pending())) { ... } Now the question here is what should 'compare' return when nothing is ready? undef? Any LDAP_ error code to indicate nothing available? -- Christophe Wolfhugel -+- chris@wolfhugel.eu

Fri Jun 07 09:04:58 2013 gbarr [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #85941] Net::LDAP 'process' method blocking in Async mode
Date:	Fri, 7 Jun 2013 08:04:41 -0500
To:	bug-perl-ldap [...] rt.cpan.org
From:	Graham Barr <gbarr [...] pobox.com>

On Jun 7, 2013, at 01:53 , Christophe Wolfhugel via RT <bug-perl-ldap@rt.cpan.org> wrote: Show quoted text

> > I do think following is a bug in 0.55 (and earlier), as this code > would block on the first loop:

That is intentional. Show quoted text

> > sub process { > my $ldap = shift; > my $what = shift; > my $sock = $ldap->socket or return LDAP_SERVER_DOWN; > my $sel = IO::Select->new($sock); > my $ready; > > for ($ready = 1 ; $ready ; $ready = $sel->can_read(0) || (ref($sock) eq 'IO::Socket::SSL' && $sock->pending())) { > my $pdu; > asn_read($sock, $pdu) > or return _drop_conn($ldap, LDAP_OPERATIONS_ERROR, 'Communications Error'); > > When calling the process method, asn_read will always be called on the > first loop, eventually blocking the process even if no messages are > available.

Well the intent was that the caller would first check that there was something ready, ie by using an event loop or something and only then would you call process. Show quoted text

> > I would believe that the loop could/should be tried as something similar to: > > while (defined $sel->can_read(0) || (ref($sock) eq 'IO::Socket::SSL' && $sock->pending())) { > ... > } > > Now the question here is what should 'compare' return when nothing is > ready? undef? Any LDAP_ error code to indicate nothing available?

No there is not, thats why it is up to the application to check first. I believe that process blocking is the correct thing. As we have to deal with SSL sockets and normal sockets, maybe we should break out the code to check if there is data ready sub data_ready { my $self = shift; my $sock = $ldap->socket or return; my $sel = IO::Select->new($sock); return defined $sel->can_read(0) || (ref($sock) eq 'IO::Socket::SSL' && $sock->pending()); } this may make things easier for those not using an event loop Graham.

Fri Jun 07 09:04:58 2013 The RT System itself - Status changed from 'new' to 'open'

Fri Jun 07 09:09:32 2013 chris [...] wolfhugel.eu - Correspondence added

Subject:	Re: [rt.cpan.org #85941] Net::LDAP 'process' method blocking in Async mode
Date:	Fri, 07 Jun 2013 14:09:09 +0100
To:	bug-perl-ldap [...] rt.cpan.org
From:	Christophe Wolfhugel <chris [...] wolfhugel.eu>

I do like the idea for having a data_read method, as you've guessed it this does indeed provide the trigger I do need in the event loop for handling the asynchronous responses when they're ready. I would guess this bug can be turned into a RFE for the data_read method. Thank you. -- Christophe Wolfhugel -+- chris@wolfhugel.eu

Sat Jun 08 11:01:49 2013 MARSCHAP [...] cpan.org - Status changed from 'open' to 'patched'

Sat Jun 08 11:01:49 2013 MARSCHAP [...] cpan.org - Severity Wishlist added

Sun Jun 09 02:49:46 2013 MARSCHAP [...] cpan.org - Status changed from 'patched' to 'resolved'

Sun Jun 09 02:49:46 2013 MARSCHAP [...] cpan.org - Fixed in 0.56 added