You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2015/10/07 00:38:28 UTC
svn commit: r1707163 - /httpd/httpd/trunk/server/util_filter.c
Author: minfrin
Date: Tue Oct 6 22:38:28 2015
New Revision: 1707163
URL: http://svn.apache.org/viewvc?rev=1707163&view=rev
Log:
Make sure that transient buckets are morphed into real buckets before
the filter returns.
Modified:
httpd/httpd/trunk/server/util_filter.c
Modified: httpd/httpd/trunk/server/util_filter.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_filter.c?rev=1707163&r1=1707162&r2=1707163&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util_filter.c (original)
+++ httpd/httpd/trunk/server/util_filter.c Tue Oct 6 22:38:28 2015
@@ -736,6 +736,16 @@ AP_DECLARE(apr_status_t) ap_filter_setas
/* decide what pool we setaside to, request pool or deferred pool? */
if (f->r) {
+ apr_bucket *e;
+ for (e = APR_BRIGADE_FIRST(bb); e != APR_BRIGADE_SENTINEL(bb); e =
+ APR_BUCKET_NEXT(e)) {
+ if (APR_BUCKET_IS_TRANSIENT(e)) {
+ int rv = apr_bucket_setaside(e, f->r->pool);
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+ }
+ }
pool = f->r->pool;
APR_BRIGADE_CONCAT(f->bb, bb);
}
Re: svn commit: r1707163 - /httpd/httpd/trunk/server/util_filter.c
Posted by Graham Leggett <mi...@sharp.fm>.
On 07 Oct 2015, at 1:54 AM, Yann Ylavic <yl...@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
—
Re: svn commit: r1707163 - /httpd/httpd/trunk/server/util_filter.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Oct 7, 2015 at 12:38 AM, <mi...@apache.org> wrote:
> Author: minfrin
> Date: Tue Oct 6 22:38:28 2015
> New Revision: 1707163
>
> URL: http://svn.apache.org/viewvc?rev=1707163&view=rev
> Log:
> Make sure that transient buckets are morphed into real buckets before
> the filter returns.
>
> Modified:
> httpd/httpd/trunk/server/util_filter.c
>
> Modified: httpd/httpd/trunk/server/util_filter.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_filter.c?rev=1707163&r1=1707162&r2=1707163&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_filter.c (original)
> +++ httpd/httpd/trunk/server/util_filter.c Tue Oct 6 22:38:28 2015
> @@ -736,6 +736,16 @@ AP_DECLARE(apr_status_t) ap_filter_setas
>
> /* decide what pool we setaside to, request pool or deferred pool? */
> if (f->r) {
> + apr_bucket *e;
> + for (e = APR_BRIGADE_FIRST(bb); e != APR_BRIGADE_SENTINEL(bb); e =
> + APR_BUCKET_NEXT(e)) {
> + if (APR_BUCKET_IS_TRANSIENT(e)) {
> + int rv = apr_bucket_setaside(e, f->r->pool);
> + if (rv != APR_SUCCESS) {
> + return rv;
> + }
> + }
> + }
> pool = f->r->pool;
> APR_BRIGADE_CONCAT(f->bb, bb);
> }
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).
Regards,
Yann.