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;
}
}
