Re: svn commit: r371484 - /httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c

[Joe Orton <jo...@redhat.com>]

On Mon, Jan 23, 2006 at 08:04:02AM -0000, Brian Pane wrote:
> Author: brianp
> Date: Mon Jan 23 00:04:01 2006
> New Revision: 371484
> 
> URL: http://svn.apache.org/viewcvs?rev=371484&view=rev
> Log:
> Return APR_EAGAIN instead of SSL_ERROR_WANT_READ from the mod_ssl filters;
> the httpd core and other modules' filters don't know what SSL_ERROR_* means.

That does not look right.  status is an apr_status_t value in this 
function; if it is ending up as an SSL_ERROR_* error code then something 
has gone wrong somewhere else.

joe

> 
> Modified:
>     httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c
> 
> Modified: httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c
> URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c?rev=371484&r1=371483&r2=371484&view=diff
> ==============================================================================
> --- httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c (original)
> +++ httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c Mon Jan 23 00:04:01 2006
> @@ -870,6 +870,8 @@
>              bucket = HTTP_ON_HTTPS_PORT_BUCKET(f->c->bucket_alloc);
>              break;
>  
> +      case SSL_ERROR_WANT_READ:
> +            return APR_EAGAIN;
>        default:
>          return status;
>      }
> 

Re: svn commit: r371484 - /httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Justin Erenkrantz wrote:
> On 2/12/06, Brian Pane <br...@apache.org> wrote:
> 
>>to return different values for "EAGAIN and please do a poll on
>>readability for me" vs. "EAGAIN and please do a poll on writability
>>for me."  The connection state logic in the event MPM assumes
>>that an EAGAIN result from an input filter means "poll for readability,"
>>but in the case of SSL that's not necessarily true.
> 
> 
> Why can't it just poll for read/write if its an SSL socket?  -- justin

Because as soon as the socket is free to write, poll will succeed.

Bill

Re: svn commit: r371484 - /httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c

[Justin Erenkrantz <ju...@erenkrantz.com>]

On 2/12/06, Brian Pane <br...@apache.org> wrote:
> to return different values for "EAGAIN and please do a poll on
> readability for me" vs. "EAGAIN and please do a poll on writability
> for me."  The connection state logic in the event MPM assumes
> that an EAGAIN result from an input filter means "poll for readability,"
> but in the case of SSL that's not necessarily true.

Why can't it just poll for read/write if its an SSL socket?  -- justin

Re: svn commit: r371484 - /httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c

[Brian Pane <br...@apache.org>]

On Jan 30, 2006, at 4:05 AM, Joe Orton wrote:

> On Mon, Jan 23, 2006 at 08:04:02AM -0000, Brian Pane wrote:
>> Author: brianp
>> Date: Mon Jan 23 00:04:01 2006
>> New Revision: 371484
>>
>> URL: http://svn.apache.org/viewcvs?rev=371484&view=rev
>> Log:
>> Return APR_EAGAIN instead of SSL_ERROR_WANT_READ from the mod_ssl  
>> filters;
>> the httpd core and other modules' filters don't know what  
>> SSL_ERROR_* means.
>
> That does not look right.  status is an apr_status_t value in this
> function; if it is ending up as an SSL_ERROR_* error code then  
> something
> has gone wrong somewhere else.

I agree; the fact that status==SSL_ERROR_WANT_READ is probably a
bad thing.  I think the only reason this hasn't been a problem in the  
past
is that the mod_ssl input filter has always done blocking reads.   
With the
nonblocking reads on the async-read-dev branch, it's now seeing EAGAIN
and (somewhere) converting that into SSL_ERROR_WANT_READ.

Thinking about it further, this seems to expose a design flaw in my
async read approach: it's possible for the mod_ssl code to hit EAGAIN
on either a read attempt or a write attempt during the connection
establishment, but there's no way in the current filter API for a filter
to return different values for "EAGAIN and please do a poll on
readability for me" vs. "EAGAIN and please do a poll on writability
for me."  The connection state logic in the event MPM assumes
that an EAGAIN result from an input filter means "poll for readability,"
but in the case of SSL that's not necessarily true.

Anybody have ideas on how to solve this problem?  Creating
different status codes for APR_EAGAIN_READ and APR_EAGAIN_WRITE
would allow mod_ssl to return sufficiently detailed information to the
caller, but it would break a lot of existing code that only understands
plain old APR_EAGAIN.

Brian