httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: svn commit: r1707163 - /httpd/httpd/trunk/server/util_filter.c
Date Wed, 07 Oct 2015 09:32:49 GMT
On 07 Oct 2015, at 1:54 AM, Yann Ylavic <ylavic.dev@gmail.com> wrote:

> I don't understand this, why using ap_save_brigade() for both cases
> (i.e. setting aside or reading the other types of buckets) would lead
> to synchronous behaviour?
> It is when r->pool gets destroyed?
> If so, how is it different from the buckets currently set aside on r->pool?
> 
> We could also possibly always use the deferred_pool (moving it to
> conn_rec if we don't want one per reinstate-able filter).

It’s because of this code here:

        rv = apr_bucket_setaside(e, p);

        /* If the bucket type does not implement setaside, then
         * (hopefully) morph it into a bucket type which does, and set
         * *that* aside... */
        if (rv == APR_ENOTIMPL) {
            const char *s;
            apr_size_t n;

            rv = apr_bucket_read(e, &s, &n, APR_BLOCK_READ);
            if (rv == APR_SUCCESS) {
                rv = apr_bucket_setaside(e, p);
            }
        }

This code reads morphing buckets into RAM, and in order to not overwhelm the server the server
is forced to block to protect itself from out-of-memory.

The actual block is triggered by ap_filter_reinstate_brigade(), which sees the morphing bucket
and marks it to be flushed, unless the bucket belongs to a request, in which case it can safely
be buffered for the next pass without fiddling with the bucket at all.

The problem comes in with transient buckets that refer to memory that stops existing when
the call eventually returns. Given that transient buckets are memory resident by definition,
moving them to the heap is not an issue.

In order to understand how this works you need to look at all three of ap_filter_reinstate_brigade(),
ap_filter_should_yield() and ap_filter_setaside_brigade() in detail, as they work together.

These three functions were implemented in the core network filter years ago and the concept
is well tested. All this patch does is generalise this concept to other filters over and over
the network filter.

Regards,
Graham
—


Mime
View raw message