Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Date: Tue, 10 May 2016 14:37:47 +0200
Message-ID: <20160510143747.Horde.23bRyFNVPC38ozmanQ6X8j4@webmail.michael-kaufmann.ch>
From: Michael Kaufmann <mail@michael-kaufmann.ch>
To: dev@httpd.apache.org
Subject: Re: Detecting client aborts and stream resets
References: <20160503163109.Horde.dDuX6tl_vMFkpJE75Qne6Q0@webmail.michael-kaufmann.ch>
 <CACsi253h9r0VE0RaSS03MKBKReCv_OP6CO=V_v06s27r+ngg2g@mail.gmail.com>
 <CACD9AFB-6811-4FF0-831F-C3C25E64580E@greenbytes.de>
 <20160504224457.Horde.q1fPqaQz_n-MFDXjfchHxG9@webmail.michael-kaufmann.ch>
 <CACsi253fdOK9X0x2LHjE6iKmSfLvGFxy+L4zbRMQA3zgnEH5sg@mail.gmail.com>
In-Reply-To: <CACsi253fdOK9X0x2LHjE6iKmSfLvGFxy+L4zbRMQA3zgnEH5sg@mail.gmail.com>
User-Agent: Horde Application Framework 5
Content-Type: text/plain; charset=utf-8; format=flowed; DelSp=Yes
MIME-Version: 1.0
Content-Disposition: inline
archived-at: Tue, 10 May 2016 12:38:05 -0000

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.