You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Michael Kaufmann <ma...@michael-kaufmann.ch> on 2016/05/03 16:31:09 UTC
Detecting client aborts and stream resets
Hi all,
a content generator module can detect client aborts and stream resets
while it reads the request body. But how can it detect this
afterwards, while the response is being generated?
This is important for HTTP/2, because the client may reset a stream,
and mod_http2 needs to wait for the content generator to finish.
Therefore the content generator should stop generating the response
when it is no longer needed.
Is there any API for this? The "conn_rec->aborted" flag exists, but
which Apache function sets this flag?
If there is no API, maybe an optional function for mod_http2 would be
a solution.
Regards,
Michael
Re: Detecting client aborts and stream resets
Posted by Mike Rumph <mi...@oracle.com>.
Hello Michael.
On 5/3/2016 7:31 AM, Michael Kaufmann wrote:
> Hi all,
>
> a content generator module can detect client aborts and stream resets
> while it reads the request body. But how can it detect this
> afterwards, while the response is being generated?
>
> This is important for HTTP/2, because the client may reset a stream,
> and mod_http2 needs to wait for the content generator to finish.
> Therefore the content generator should stop generating the response
> when it is no longer needed.
>
> Is there any API for this? The "conn_rec->aborted" flag exists, but
> which Apache function sets this flag?
The conn_rec->aborted flag is set in the following functions:
- ap_core_output_filter() in server/core_filters.c
- ap_process_connection() in server/connection.c
- process_socket() in server/mpm/event/event.c
- worker_main() in server/mpm/winnt/child.c
> If there is no API, maybe an optional function for mod_http2 would be
> a solution.
>
> Regards,
> Michael
>
>
Re: Detecting client aborts and stream resets
Posted by Michael Kaufmann <ma...@michael-kaufmann.ch>.
Zitat von Stefan Eissing <st...@greenbytes.de>:
> Thanks for the patch! I applied it to trunk in r1743335, will be part of next
> 1.5.4 release. I only omitted the last change as I do not want to set aborted
> on the main connection every time the session closes.
Ok, that's fine for me. Thanks a lot Stefan!
Regards,
Michael
Re: Detecting client aborts and stream resets
Posted by Stefan Eissing <st...@greenbytes.de>.
Thanks for the patch! I applied it to trunk in r1743335, will be part of next
1.5.4 release. I only omitted the last change as I do not want to set aborted
on the main connection every time the session closes.
Cheers,
Stefan
> Am 10.05.2016 um 14:37 schrieb Michael Kaufmann <ma...@michael-kaufmann.ch>:
>
> Zitat von William A Rowe Jr <wr...@rowe-clan.net>:
>
>> On Wed, May 4, 2016 at 3:44 PM, Michael Kaufmann <ma...@michael-kaufmann.ch>
>> wrote:
>>
>>> William is right, this is not a good idea. The ->aborted flag should serve
>>>> this purpose of telling anyone interested that this connection is not
>>>> longer delivering. I will make a github release soon where that is working
>>>> and you can test.
>>>>
>>>>
>>> Thank you Stefan! It is now working for stream resets, but it's not yet
>>> working if the client closes the whole TCP connection.
>>>
>>
>> As expected... this is why I pointed out in my first reply that you don't
>> want a single-protocol solution to this puzzle.
>
> Of course I'd also prefer a general solution.
>
> I have created a patch for mod_http2: With the patch, it sets c->aborted on the master connection and also on the "dummy" connections (streams) when the client closes the TCP connection.
>
> @Stefan: It would be great if you could check this code and add it to mod_http2 :-)
>
>
> Index: modules/http2/h2_mplx.c
> ===================================================================
> --- modules/http2/h2_mplx.c (revision 1743146)
> +++ modules/http2/h2_mplx.c (working copy)
> @@ -488,6 +488,15 @@
> return 1;
> }
>
> +static int task_abort_connection(void *ctx, void *val)
> +{
> + h2_task *task = val;
> + if (task->c) {
> + task->c->aborted = 1;
> + }
> + return 1;
> +}
> +
> apr_status_t h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
> {
> apr_status_t status;
> @@ -537,6 +546,8 @@
> * and workers *should* return in a timely fashion.
> */
> for (i = 0; m->workers_busy > 0; ++i) {
> + h2_ihash_iter(m->tasks, task_abort_connection, m);
> +
> m->join_wait = wait;
> status = apr_thread_cond_timedwait(wait, m->lock, apr_time_from_sec(wait_secs));
>
> @@ -591,6 +602,7 @@
> AP_DEBUG_ASSERT(m);
> if (!m->aborted && enter_mutex(m, &acquired) == APR_SUCCESS) {
> m->aborted = 1;
> + m->c->aborted = 1;
> h2_ngn_shed_abort(m->ngn_shed);
> leave_mutex(m, acquired);
> }
>
>
>
>
>> See my later reply about detecting connection tear-down, patches welcome.
>
> Sounds good! If someone sends a patch, I will help to test it.
Re: Detecting client aborts and stream resets
Posted by Michael Kaufmann <ma...@michael-kaufmann.ch>.
Zitat von William A Rowe Jr <wr...@rowe-clan.net>:
> On Wed, May 4, 2016 at 3:44 PM, Michael Kaufmann <ma...@michael-kaufmann.ch>
> wrote:
>
>> William is right, this is not a good idea. The ->aborted flag should serve
>>> this purpose of telling anyone interested that this connection is not
>>> longer delivering. I will make a github release soon where that is working
>>> and you can test.
>>>
>>>
>> Thank you Stefan! It is now working for stream resets, but it's not yet
>> working if the client closes the whole TCP connection.
>>
>
> As expected... this is why I pointed out in my first reply that you don't
> want a single-protocol solution to this puzzle.
Of course I'd also prefer a general solution.
I have created a patch for mod_http2: With the patch, it sets
c->aborted on the master connection and also on the "dummy"
connections (streams) when the client closes the TCP connection.
@Stefan: It would be great if you could check this code and add it to
mod_http2 :-)
Index: modules/http2/h2_mplx.c
===================================================================
--- modules/http2/h2_mplx.c (revision 1743146)
+++ modules/http2/h2_mplx.c (working copy)
@@ -488,6 +488,15 @@
return 1;
}
+static int task_abort_connection(void *ctx, void *val)
+{
+ h2_task *task = val;
+ if (task->c) {
+ task->c->aborted = 1;
+ }
+ return 1;
+}
+
apr_status_t h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait)
{
apr_status_t status;
@@ -537,6 +546,8 @@
* and workers *should* return in a timely fashion.
*/
for (i = 0; m->workers_busy > 0; ++i) {
+ h2_ihash_iter(m->tasks, task_abort_connection, m);
+
m->join_wait = wait;
status = apr_thread_cond_timedwait(wait, m->lock,
apr_time_from_sec(wait_secs));
@@ -591,6 +602,7 @@
AP_DEBUG_ASSERT(m);
if (!m->aborted && enter_mutex(m, &acquired) == APR_SUCCESS) {
m->aborted = 1;
+ m->c->aborted = 1;
h2_ngn_shed_abort(m->ngn_shed);
leave_mutex(m, acquired);
}
> See my later reply about detecting connection tear-down, patches welcome.
Sounds good! If someone sends a patch, I will help to test it.
Re: Detecting client aborts and stream resets
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, May 4, 2016 at 3:44 PM, Michael Kaufmann <ma...@michael-kaufmann.ch>
wrote:
> William is right, this is not a good idea. The ->aborted flag should serve
>> this purpose of telling anyone interested that this connection is not
>> longer delivering. I will make a github release soon where that is working
>> and you can test.
>>
>>
> Thank you Stefan! It is now working for stream resets, but it's not yet
> working if the client closes the whole TCP connection.
>
As expected... this is why I pointed out in my first reply that you don't
want a single-protocol solution to this puzzle.
See my later reply about detecting connection tear-down, patches welcome.
Re: Detecting client aborts and stream resets
Posted by Michael Kaufmann <ma...@michael-kaufmann.ch>.
> William is right, this is not a good idea. The ->aborted flag should
> serve this purpose of telling anyone interested that this connection
> is not longer delivering. I will make a github release soon where
> that is working and you can test.
>
Thank you Stefan! It is now working for stream resets, but it's not
yet working if the client closes the whole TCP connection.
Regards,
Michael
Re: Detecting client aborts and stream resets
Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 03.05.2016 um 17:35 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
>
> On Tue, May 3, 2016 at 9:31 AM, Michael Kaufmann <ma...@michael-kaufmann.ch> wrote:
> Hi all,
>
> a content generator module can detect client aborts and stream resets while it reads the request body. But how can it detect this afterwards, while the response is being generated?
>
> This is important for HTTP/2, because the client may reset a stream, and mod_http2 needs to wait for the content generator to finish. Therefore the content generator should stop generating the response when it is no longer needed.
>
> Is there any API for this? The "conn_rec->aborted" flag exists, but which Apache function sets this flag?
conn_rec->aborted is currently not set by mod_http2 on slave connections, but should. I'll add that.
> If there is no API, maybe an optional function for mod_http2 would be a solution.
>
> Nope - an optional function in mod_http2 is too special case, generators
> must remain protocol (socket or other transport) agnostic.
>
> In the case of mod_cache'd content, the generator can't quit, it is already
> populating a cache, which means you'll generate an invalid cached object.
>
> But if you knew that the cache module wasn't collecting info, r->c->aborted
> tells you if anyone is still listening, right?
William is right, this is not a good idea. The ->aborted flag should serve this purpose of telling anyone interested that this connection is not longer delivering. I will make a github release soon where that is working and you can test.
-Stefan
Re: Detecting client aborts and stream resets
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, May 3, 2016 at 2:05 PM, Michael Kaufmann <ma...@michael-kaufmann.ch>
wrote:
> Zitat von William A Rowe Jr <wr...@rowe-clan.net>:
>
>>
>> Nope - an optional function in mod_http2 is too special case, generators
>> must remain protocol (socket or other transport) agnostic.
>>
>
> Sure, official Apache modules should not call protocol-dependent hooks,
> but it could be a solution for 3rd party modules.
Nonsense, third party module designers should be equally aware - when you
start meddling in unexpected edge cases, binary ABI won't be maintained
in those edge cases. You are always better off relying on the intended
design
of httpd, unless you want to chase a moving target.
Extending this detection with a protocol-agnostic API would be interesting,
however an optional function has only a single registered provider, so it
would
be the wrong interface. You want an interface that would test/report "gone
away"
irrespective of http1 vs http2 vs future protocols.
> In the case of mod_cache'd content, the generator can't quit, it is already
>> populating a cache, which means you'll generate an invalid cached object.
>>
>> But if you knew that the cache module wasn't collecting info,
>> r->c->aborted
>> tells you if anyone is still listening, right?
>>
>
> Unfortunately not. While the content generator is running (just preparing
> the response, it is not reading from the input filters anymore and not
> writing to the output filters yet), Apache does not check the connection's
> state.
>
Good point... where/how would we address this for CPU intensive generators?
We certainly don't bother in proxy, because proxy is much lighter-weight.
The obvious answer to me in a worker or event MPM would be for the user
to make a 'notify close?' request to the MPM, which would add the socket
with for the POLLHUP event alone after reading the input brigade (the read
will signal r->c->aborted already), with the single goal of setting the flag
r->c->aborted upon socket close. Very lightweight, but that presumes that
a second poll on the same fd for different events (e.g. poll on read or
write)
elsewhere won't give the kernel conniptions.
mod_http2 has enough going on the master connection that detecting the
stream close should occur fairly promptly.
Re: Detecting client aborts and stream resets
Posted by Michael Kaufmann <ma...@michael-kaufmann.ch>.
Zitat von William A Rowe Jr <wr...@rowe-clan.net>:
> On Tue, May 3, 2016 at 9:31 AM, Michael Kaufmann <ma...@michael-kaufmann.ch>
> wrote:
>
>> Hi all,
>>
>> a content generator module can detect client aborts and stream resets
>> while it reads the request body. But how can it detect this afterwards,
>> while the response is being generated?
>>
>> This is important for HTTP/2, because the client may reset a stream, and
>> mod_http2 needs to wait for the content generator to finish. Therefore the
>> content generator should stop generating the response when it is no longer
>> needed.
>>
>> Is there any API for this? The "conn_rec->aborted" flag exists, but which
>> Apache function sets this flag?
>>
>> If there is no API, maybe an optional function for mod_http2 would be a
>> solution.
>>
>
> Nope - an optional function in mod_http2 is too special case, generators
> must remain protocol (socket or other transport) agnostic.
>
Sure, official Apache modules should not call protocol-dependent
hooks, but it could be a solution for 3rd party modules.
> In the case of mod_cache'd content, the generator can't quit, it is already
> populating a cache, which means you'll generate an invalid cached object.
>
> But if you knew that the cache module wasn't collecting info, r->c->aborted
> tells you if anyone is still listening, right?
Unfortunately not. While the content generator is running (just
preparing the response, it is not reading from the input filters
anymore and not writing to the output filters yet), Apache does not
check the connection's state.
I'm not sure how mod_proxy deals with this - does it abort the backend
request when the client closes the connection?
Re: Detecting client aborts and stream resets
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, May 3, 2016 at 9:31 AM, Michael Kaufmann <ma...@michael-kaufmann.ch>
wrote:
> Hi all,
>
> a content generator module can detect client aborts and stream resets
> while it reads the request body. But how can it detect this afterwards,
> while the response is being generated?
>
> This is important for HTTP/2, because the client may reset a stream, and
> mod_http2 needs to wait for the content generator to finish. Therefore the
> content generator should stop generating the response when it is no longer
> needed.
>
> Is there any API for this? The "conn_rec->aborted" flag exists, but which
> Apache function sets this flag?
>
> If there is no API, maybe an optional function for mod_http2 would be a
> solution.
>
Nope - an optional function in mod_http2 is too special case, generators
must remain protocol (socket or other transport) agnostic.
In the case of mod_cache'd content, the generator can't quit, it is already
populating a cache, which means you'll generate an invalid cached object.
But if you knew that the cache module wasn't collecting info, r->c->aborted
tells you if anyone is still listening, right?