Skip to content

Make get_request timeouts recoverable #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Juerd
Copy link

@Juerd Juerd commented Mar 3, 2015

With short select() delays (e.g. in event loops) and slow connections,
the client may not have more data ready before select timeouts.

This change lets get_request return a different kind of false (defined
instead of undef) in case of a select timeout, so that the caller can
choose to call it again. To make this work, the same buffer
is re-used with _need_more every time.

Hacks that inherit from HTTP::Daemon and override _need_more, will
break, as will existing code that explicitly tests the result of
get_request for definedness rather than its boolean value, and then
assumes that it will be an HTTP::Request object. Both cases are
unlikely.

The unit test is based on chunked.t with a lot removed.

With short select() delays (e.g. in event loops) and slow connections,
the client may not have more data ready before select timeouts.

This change lets get_request return a different kind of false (defined
instead of undef) in case of a select timeout, so that the caller can
choose to call it again. To make this work, the same buffer
is re-used with _need_more every time.

Hacks that inherit from HTTP::Daemon and override _need_more, will
break, as will existing code that explicitly tests the result of
get_request for definedness rather than its boolean value, and then
assumes that it will be an HTTP::Request object. Both cases are
unlikely.

The unit test is based on chunked.t with a lot removed.
@midenok
Copy link

midenok commented Jul 1, 2016

Why you need two select()'s: first one in your event loop and second one in _need_more()? select() in _need_more() is the implementation of timeout logic which is not needed in event loop (as far as you've added HTTP::Daemon::ClientConn fd to it). Instead of vague return type and small timeout hack you should avoid select() in _need_more() completely and check for EAGAIN after sysread(). There should be some option to tell HTTP::Daemon to don't do select(), e.g.:

  $d = HTTP::Daemon->new(
           NonBlock => 1
       );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants