httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/ modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/ server/mpm/simple/
Date Tue, 06 Oct 2015 10:36:32 GMT
On Tue, Oct 6, 2015 at 11:31 AM, Plüm, Rüdiger, Vodafone Group
<ruediger.pluem@vodafone.com> wrote:
>
>
>> -----Original Message-----
>> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>> Sent: Dienstag, 6. Oktober 2015 11:13
>> To: dev@httpd.apache.org
>> Subject: Re: svn commit: r1706669 - in /httpd/httpd/trunk: ./ include/
>> modules/http/ modules/ssl/ server/ server/mpm/event/ server/mpm/motorz/
>> server/mpm/simple/
>>
>> On Tue, Oct 6, 2015 at 9:37 AM, Plüm, Rüdiger, Vodafone Group
>> <ruediger.pluem@vodafone.com> wrote:
>> >
>> >> -----Original Message-----
>> >> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>> >>
>> >> > Modified: httpd/httpd/trunk/server/mpm/event/event.c
>> >> > URL:
>> >>
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?re
>> >> v=1706669&r1=1706668&r2=1706669&view=diff
>> >> >
>> >>
>> ==========================================================================
>> >> ====
>> >> > --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>> >> > +++ httpd/httpd/trunk/server/mpm/event/event.c Sun Oct  4 10:10:51
>> 2015
>> >> > @@ -1146,19 +1146,38 @@ read_request:
>> >> >      }
>> >> >
>> >> []
>> >> > +
>> >> > +        rindex = apr_hash_first(NULL, c->filters);
>> >> > +        while (rindex) {
>> >> > +            ap_filter_t *f = apr_hash_this_val(rindex);
>> >> > +
>> >> > +            if (!APR_BRIGADE_EMPTY(f->bb)) {
>> >> > +
>> >> > +                rv = ap_pass_brigade(f, c->empty);
>> >> > +                apr_brigade_cleanup(c->empty);
>> >> > +                if (APR_SUCCESS != rv) {
>> >> > +                    ap_log_cerror(
>> >> > +                            APLOG_MARK, APLOG_DEBUG, rv, c,
>> >> APLOGNO(00470)
>> >> > +                            "write failure in '%s' output filter",
>> f-
>> >> >frec->name);
>> >> > +                    break;
>> >> > +                }
>> >> > +
>> >> > +                if (ap_filter_should_yield(f)) {
>> >> > +                    data_in_output_filters = 1;
>> >> > +                }
>> >> > +            }
>> >> > +
>> >> > +            rindex = apr_hash_next(rindex);
>> >> >          }
>> >>
>> >> This pattern looks shared by all (relevant) MPMs, couldn't we make it
>> >> a function? Maybe ap_reinstate_conn()?
>> >
>> > Sounds sensible.
>>
>> By defining this function in util_filter
>> (ap_filter_reinstate_conn()?), we also possibly could get rid of
>> c->empty (which looks quite hacky), using/hidding an empty brigade per
>> filter (in opaque data).
>
> Although the brigade is empty, don't we need to prevent the brigade being used by multiple
threads in parallel?
> Using one per connection prevents this.

Hmm, the filters are also allocated per connection/request (on
c/r->pool), and shouldn't be called concurrently right?

BTW, I wonder if we can "reinstate" the filters in arbitrary order
like in the above loop (the order seems to depend on the calls to
ap_filter_setaside_brigade() and internal apr_hash_t ordering).
Don't we need to start from r->ouput_filters down to the last filter
and call ap_pass_brigade(f, c->empty) if f is in the hashtable?

Mime
View raw message