apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jerenkra...@ebuilt.com>
Subject Re: cvs commit: apr-util/buckets apr_buckets_socket.c
Date Wed, 10 Oct 2001 05:44:56 GMT
On Tue, Oct 09, 2001 at 10:35:15AM -0400, Cliff Woolley wrote:
> -1.  By doing this, any time you do a nonblocking read from a socket
> bucket and get EAGAIN, the whole bucket vaporizes and you never get the
> chance to read any more data.  It was correct before: if you ask for a
> nonblocking read, you have to be ready to handle an EAGAIN case.  So we

Yes, I didn't think of that case - you are right - it probably
explains some of the problems I was seeing with mod_ssl.

I'm not sure that the bucket read code should be returning an error 
on anything but fatal errors.  And, receiving EAGAIN isn't a fatal 
error.  My take is that forcing the caller of bucket_read to handle
EAGAIN isn't very clean.  I think this breaks the abstraction between 
sockets and buckets - I'm not sure that the caller of the bucket_read
needs to handle EAGAIN explicitly.  Isn't part of the idea of buckets
is to abstract out certain implementation details - the caller shouldn't
know that it has a socket bucket underneath it?

Has this been decided/discussed before?  If so, then I'll just shut 
up.  =)

Considering the above comments, would this patch satisfy your 
veto?  -- justin

Index: buckets/apr_buckets_socket.c
RCS file: /home/cvs/apr-util/buckets/apr_buckets_socket.c,v
retrieving revision 1.32
diff -u -r1.32 apr_buckets_socket.c
--- buckets/apr_buckets_socket.c	2001/10/09 05:17:18	1.32
+++ buckets/apr_buckets_socket.c	2001/10/10 05:26:29
@@ -77,10 +77,14 @@
     if (block == APR_NONBLOCK_READ) {
         apr_setsocketopt(p, APR_SO_TIMEOUT, timeout);
         /* There was nothing to read right now, so treat it as okay and 
-         * return a 0-length brigade (see below). */
+         * return a 0-length bucket. */
         if (APR_STATUS_IS_EAGAIN(rv)) {
+            free(buf);
             *len = 0;
-            rv = APR_SUCCESS;
+            a = apr_bucket_immortal_make(a, "", 0);
+            *str = a->data;
+            APR_BUCKET_INSERT_AFTER(a, apr_bucket_socket_create(p));
+            return APR_SUCCESS;

View raw message