httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Eissing <stefan.eiss...@greenbytes.de>
Subject Re: Detecting client aborts and stream resets
Date Wed, 11 May 2016 11:05:40 GMT
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 <mail@michael-kaufmann.ch>:
> 
> Zitat von William A Rowe Jr <wrowe@rowe-clan.net>:
> 
>> On Wed, May 4, 2016 at 3:44 PM, Michael Kaufmann <mail@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.


Mime
View raw message