On 3/31/20 6:22 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Tue Mar 31 16:22:53 2020
> New Revision: 1875947
>
> URL: http://svn.apache.org/viewvc?rev=1875947&view=rev
> Log:
> core: handle morphing buckets setaside/reinstate and kill request core filter.
>
> The purpose of ap_request_core_filter() is not clear, it seems to prevent
> potential morphing buckets to go through AP_FTYPE_CONNECTION filters which
> would fail to set them aside (ENOTIMPL), and read them (unbounded) in memory.
>
> This patch allows ap_filter_setaside_brigade() to set morphing buckets aside
> by simply moving them, assuming they have the correct lifetime (either until
> some further EOR, or the connection lifetime, or whatever). IOW, the module is
> responsible for sending morphing buckets whose lifetime needs not be changed
> by the connection filters.
>
> Now since morphing buckets consume no memory until (apr_bucket_)read, like
> FILE
> buckets, we don't account for them in flush_max_threshold either. This changes
> ap_filter_reinstate_brigade() to only account for in-memory and EOR buckets to
> flush_upto.
>
> Also, since the EOR bucket is sent only to c->output_filters once the request
> is processed, when all the filters < AP_FTYPE_CONNECTION have done their job
> and stopped retaining data (after the EOS bucket, if ever), we prevent misuse
> of ap_filter_{setaside,reinstate}_brigade() outside connection filters by
> returning ENOTIMPL. This is not the right API for request filters as of now.
>
> Finally, ap_request_core_filter() and co can be removed.
>
> Modified:
> httpd/httpd/trunk/include/http_request.h
> httpd/httpd/trunk/modules/http/http_core.c
> httpd/httpd/trunk/modules/http/http_request.c
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/core_filters.c
> httpd/httpd/trunk/server/request.c
> httpd/httpd/trunk/server/util_filter.c
>
> Modified: httpd/httpd/trunk/include/http_request.h
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_request.h?rev=1875947&r1=1875946&r2=1875947&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/http_request.h (original)
> +++ httpd/httpd/trunk/include/http_request.h Tue Mar 31 16:22:53 2020
> @@ -601,6 +589,15 @@ AP_DECLARE(int) ap_if_walk(request_rec *
> AP_DECLARE_DATA extern const apr_bucket_type_t ap_bucket_type_eor;
>
> /**
> + * Determine if a bucket is morphing, that is which changes its
> + * type on read (usually to "heap" allocated data), while moving
> + * itself at the next position to remain plugged until exhausted.
> + * @param e The bucket to inspect
> + * @return true or false
> + */
> +#define AP_BUCKET_IS_MORPHING(e) ((e)->length == (apr_size_t)-1)
Nitpick: After having a second thought on the whole thing, I think the above
name is misleading to some extend. If MMAP is enabled
a file bucket is also a morphing bucket as a read on it causes the bucket to
split in an MMAP bucket and a shorter file bucket.
A MMAP bucket consumes at least address space and I vaguely remember cases from
the past (back in 32 bit times) where filters that
processed (doing apr_bucket_read) a whole brigade at once without passing
results down the chain on a regular basis caused the
address space to be exhausted by MMAP buckets if the file bucket was huge.
> +
> +/**
> * Determine if a bucket is an End Of REQUEST (EOR) bucket
> * @param e The bucket to inspect
> * @return true or false
>
Regards
Rüdiger
