Ack. Thanks for confirming that the behavior should be loop related. I will try to find out how this work in the context loop where we have read buckets in multiple allocators.
It should also include that ssl related extra ‘eagain’ value for stop writing. I’m not sure if the current status improved, but we currently trip that check far too many times. It is hard to tell when this problem was introduced, given that we broke compilation with this option quite some time ago. Some of the error instances are related to having a peek on an outer bucket cause a read on an inner bucket, while the result of an error peek is not always forwarded to the true error handling. The EAGAIN cases might be related, but I started by disabling those to fix the bigger problems first. Bert Sent from Mail for Windows 10 From: Greg Stein Sent: donderdag 12 november 2015 01:44 To: Bert Huijben Cc: [email protected] Subject: Re: svn commit: r1713936 - in /serf/trunk: buckets/allocator.cbuckets/log_wrapper_buckets.c outgoing.c test/mock_buckets.c On Wed, Nov 11, 2015 at 3:29 PM, <[email protected]> wrote: >... > +++ serf/trunk/buckets/allocator.c Wed Nov 11 21:29:45 2015 > @@ -416,9 +416,11 @@ apr_status_t serf_debug__record_read( > read_status_t *rs = find_read_status(track, bucket, 1); > > /* Validate that the previous status value allowed for another read. > */ > - if (APR_STATUS_IS_EAGAIN(rs->last) /* ### or APR_EOF? */) { > + if (SERF_BUCKET_READ_ERROR(rs->last)) { > This is wrong. If a bucket returns APR_EAGAIN, you are *not* supposed to read the bucket again until the context loop has been invoked. I agree that you should stop reading if a READ_ERROR occurs. That is a good addition. But that excludes EAGAIN, incorrectly. >... > @@ -492,6 +497,27 @@ void serf_debug__bucket_destroy(const se > if (SERF_BUCKET_IS_BARRIER(bucket)) > return; > > + if (SERF_BUCKET_IS_AGGREGATE(bucket)) { > + apr_status_t status; > + const char *data; > + apr_size_t len; > + > + serf_bucket_aggregate_hold_open(bucket, NULL, NULL); > + > + status = serf_bucket_read(bucket, SERF_READ_ALL_AVAIL, > + &data, &len); > + > + if (APR_STATUS_IS_EOF(status) && !len) > + return; > + } > + > + bkt = serf_bucket_read_bucket((serf_bucket_t*)bucket, > + &serf_bucket_type_ssl_encrypt); > + > + if (bkt != NULL) { > + serf_bucket_destroy(bkt); > + return; > + } > What is all that?? How about some comments, please. >... Cheers, -g
