httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bri...@apache.org>
Subject Re: svn commit: r371484 - /httpd/httpd/branches/async-read-dev/modules/ssl/ssl_engine_io.c
Date Mon, 13 Feb 2006 05:35:05 GMT
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


Mime
View raw message