httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r921378 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_reqtimeout.c
Date Mon, 15 Mar 2010 07:28:45 GMT
On 12.03.2010 20:47, Stefan Fritsch wrote:
> On Fri, 12 Mar 2010, Ruediger Pluem wrote:
>>> @@ -114,6 +105,24 @@ static apr_status_t reqtimeout_filter(ap
>>>          return ap_get_brigade(f->next, bb, mode, block, readbytes);
>>>      }
>>>
>>> +    if (!ccfg->socket) {
>>> +        core_net_rec *net_rec;
>>> +        ap_filter_t *core_in = f->next;
>>> +
>>> +        while (core_in && core_in->frec != ap_core_input_filter_handle)
>>> +            core_in = core_in->next;
>>> +
>>> +        if (!core_in) {
>>> +            ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, f->c,
>>> +                          "mod_reqtimeout: Can't get socket "
>>> +                          "handle from core_input_filter");
>>> +            ap_remove_input_filter(f);
>>> +            return ap_get_brigade(f->next, bb, mode, block, readbytes);
>>> +        }
>>> +        net_rec = core_in->ctx;
>>> +        ccfg->socket = net_rec->client_socket;
>>> +    }
>>> +
>>
>> Hm, this looks kind of ugly. Why not leaving things in
>> r->connection->conn_config and run through both hooks (pre_connection and
>> process_connection). The first one stores the socket, the second one
>> applies
>> the filter if ever reached.
> 
> I wanted to avoid allocating the memory for mod_reqtimeout's conn_config
> if it is not enabled anyway. Without the conn_config, there is no place
> to store the socket. If you think wasting a bit of memory is better than
> that loop, I am fine with that too. But usually there are only very few

I think wasting this small amount of memory is ok and if someone does not
want mod_reqtimeout he should simply not load it and in this case there
is no waste at all.

> connection input filters and the chain we have to traverse is rather
> short. Actually I don't know of any module that sits between
> mod_reqtimeout and the core_input_filter, even mod_ssl is behind
> mod_reqtimeout.
> 
> I have also tried inserting the filter in pre_connection and storing the
> socket as filter context, and then removing the filter in the first
> invocation of reqtimeout_filter. But this did not work well either,
> because ap_remove_input_filter can't remove a connection filter from the
> current request (is this a bug?).

Hm, that sounds like a bug, but no time to dig deeper.

Regards

RĂ¼diger


Mime
View raw message