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