Bug 56333 - API to resume a SUSPENDED connection
Summary: API to resume a SUSPENDED connection
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mpm_event (show other bugs)
Version: 2.5-HEAD
Hardware: PC All
: P2 enhancement with 6 votes (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: FixedInTrunk, PatchAvailable
Depends on:
Blocks:
 
Reported: 2014-03-31 20:38 UTC by Artem
Modified: 2014-05-11 20:53 UTC (History)
1 user (show)



Attachments
Patch for httpd-2.4.9 implementing `ap_resume_suspended`. (1.72 KB, patch)
2014-04-12 08:52 UTC, Artem
Details | Diff
Test case working with httpd-2.4.9. (2.90 KB, text/plain)
2014-04-12 08:55 UTC, Artem
Details
Test case working with httpd-2.4.9. (2.67 KB, text/plain)
2014-04-13 12:47 UTC, Artem
Details
Add support for connection suspension in Event MPM (8.08 KB, patch)
2014-04-22 20:29 UTC, Edward Lu
Details | Diff
Testcase for new patch (3.09 KB, text/plain)
2014-04-22 20:31 UTC, Edward Lu
Details
Add support for resuming a suspended connection in Event MPM (8.09 KB, patch)
2014-04-23 09:18 UTC, Artem
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Artem 2014-03-31 20:38:08 UTC
When using SUSPENDED connections to serve HTTP request the connection is never closed. For example, as discussed at http://thread.gmane.org/gmane.comp.apache.user/104996, this test case https://gist.github.com/ArtemGr/9870554 will leak connections and will fail an "ab" test.

Please introduce an API allowing one to return a SUSPENDED connection back to the mpm_event queue, so that the mpm_event will properly flush and close the connection (or even reuse it with KeepAlive).
A rough example of such an API was hacked together in the following patch: https://gist.github.com/ArtemGr/9887564 (for httpd 2.4.6).
Comment 1 Artem 2014-04-12 03:56:33 UTC
FYI: In httpd 2.4.9 my hack no longer works for some reason.
Comment 2 Artem 2014-04-12 08:52:07 UTC
Created attachment 31519 [details]
Patch for httpd-2.4.9 implementing `ap_resume_suspended`.

A (hopefully improved) version of the patch.
Comment 3 Artem 2014-04-12 08:55:27 UTC
Created attachment 31520 [details]
Test case working with httpd-2.4.9.

Nevermind my previous comment about patch no longer working with httpd-2.4.9.
It works (if ap_mpm_register_timed_callback isn't used).

Attached is the test case.
Without the patch it hangs on "ab" and with the patch it works NP.
Comment 4 Artem 2014-04-13 12:44:41 UTC
Comment on attachment 31520 [details]
Test case working with httpd-2.4.9.

>// apxs -i -a -c mod_sustest.c
>// SetHandler sustest
>// ab -n 9999 -c 9 http://localhost/sustest
>
>#include <httpd.h>
>#include <http_protocol.h>
>#include <http_config.h>
>#include <http_request.h>
>#include <http_log.h>
>#include <http_connection.h>
>#include <ap_mpm.h>
>
>#include <unistd.h>
>
>module sustest_module;
>APLOG_USE_MODULE (sustest);
>
>static void render (request_rec* r) {
>  r->content_type = "text/html; charset=utf-8";
>  if (!r->header_only) {
>    ap_rputs ("<!doctype html><html lang=\"ru\"><head><meta charset=\"UTF-8\"><title>test</title></head><body>\n", r);
>    if (r->filename) ap_rprintf (r, "FS path: %s<br/>\n", r->filename);
>    ap_rputs ("v. 02<br/>", r);
>    ap_rputs ("</body></html>\n", r);
>  }
>}
>
>static void setupSuspended (request_rec* r) {
>}
>
>void ap_resume_suspended (conn_rec* c);
>
>static void finishSuspended (request_rec* r) {
>  conn_rec* c = r->connection;
>  apr_thread_mutex_lock (r->invoke_mtx);
>
>  if (r->connection->aborted) {
>    ap_die (HTTP_INTERNAL_SERVER_ERROR, r);
>  } else if (r->status == HTTP_OK) {
>    apr_bucket_brigade* bb = apr_brigade_create (r->pool, r->connection->bucket_alloc);
>    //APR_BRIGADE_INSERT_TAIL (bb, apr_bucket_flush_create (r->connection->bucket_alloc));
>    APR_BRIGADE_INSERT_TAIL (bb, apr_bucket_eos_create (r->connection->bucket_alloc));
>    ap_pass_brigade (r->output_filters, bb);
>    apr_brigade_destroy (bb);
>    ap_finalize_request_protocol (r);
>  }
>
>  apr_thread_mutex_unlock (r->invoke_mtx);
>  ap_process_request_after_handler (r);
>  ap_log_rerror (APLOG_MARK, LOG_INFO, r->status, r, "Calling resume_suspended...");
>  ap_resume_suspended (c);
>}
>
>static void callback (void* vr) {
>  request_rec* r = (request_rec*) vr;
>  ap_log_rerror (APLOG_MARK, LOG_INFO, r->status, r, "Callback.");
>
>  if (!r->connection->aborted) render (r);
>
>  finishSuspended (r);
>}
>
>static void* thread_func (void* vr) {
>  usleep (100);
>  callback (vr);
>  return NULL;
>}
>
>static int sustest_handler (request_rec* r) {
>  if (!r->handler || strcmp (r->handler, "sustest")) return DECLINED;
>  if (ap_meets_conditions (r) != OK) {ap_log_rerror (APLOG_MARK, APLOG_NOTICE, 0, r, "!ap_meets_conditions"); return DECLINED;}
>
>  ap_log_rerror (APLOG_MARK, LOG_INFO, r->status, r, "sustest_handler");
>  //render (r); return OK;
>  setupSuspended (r);
>
>  pthread_t thread;
>  if (pthread_create (&thread, NULL, thread_func, r) != 0) return DECLINED;
>  pthread_detach (thread);
>  //ap_mpm_register_timed_callback (apr_time_from_msec (90), callback, r);
>  return SUSPENDED;
>}
>
>static void sustest_hooks (apr_pool_t *pool) {
>  ap_hook_handler (sustest_handler, NULL, NULL, APR_HOOK_FIRST);
>}
>
>module sustest_module = {
>  STANDARD20_MODULE_STUFF,
>  NULL,
>  NULL,
>  NULL,
>  NULL,
>  NULL,
>  sustest_hooks
>};
Comment 5 Artem 2014-04-13 12:47:29 UTC
Created attachment 31521 [details]
Test case working with httpd-2.4.9.

(Keep-alive works fine with the patch).
Comment 6 Edward Lu 2014-04-22 20:29:24 UTC
Created attachment 31548 [details]
Add support for connection suspension in Event MPM

Cleaned up the code; I changed the globally exposed function to a hook, modeled after the timed_callback which is very similar. The code within the hook is yours, though.

In doing so, I had to introduce a new ap_mpm_query() query, to make it worker/prefork-safe. Any code calling the new function should check if the MPM supports suspension first.
Comment 7 Edward Lu 2014-04-22 20:31:41 UTC
Created attachment 31549 [details]
Testcase for new patch

Forgot to mention that the new patch is against trunk, not against 2.4.9.

Here's the testcase, changed a little for the new patch.
Comment 8 Artem 2014-04-23 09:18:03 UTC
Created attachment 31551 [details]
Add support for resuming a suspended connection in Event MPM

Fixed the error messages in the Edward Lu's patch to match the new function name.
Comment 9 Artem 2014-04-23 14:00:56 UTC
Edward, thanks for looking at this!
Comment 10 Eric Covener 2014-05-11 20:53:47 UTC
http://svn.apache.org/r1593860