On Sun, Nov 3, 2013 at 8:37 PM, <cove...@apache.org> wrote:
> Author: covener
> Date: Mon Nov 4 01:37:31 2013
> New Revision: 1538490
>
> URL: http://svn.apache.org/r1538490
> Log:
> c->sbh can be unexpectedly NULL when the thread that pulls the ready
> keepalive
> connection out of the queue laps the thread that put it on the queue.
>
>
> Modified:
> httpd/httpd/trunk/server/mpm/event/event.c
> httpd/httpd/trunk/server/mpm/eventopt/eventopt.c
>
> Modified: httpd/httpd/trunk/server/mpm/event/event.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1538490&r1=1538489&r2=1538490&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Mon Nov 4 01:37:31 2013
> @@ -1066,6 +1066,7 @@ read_request:
> */
> cs->expiration_time = ap_server_conf->keep_alive_timeout +
> apr_time_now();
> + c->sbh = NULL;
> apr_thread_mutex_lock(timeout_mutex);
> TO_QUEUE_APPEND(keepalive_q, cs);
>
> @@ -1079,6 +1080,7 @@ read_request:
> "process_socket: apr_pollset_add failure");
> AP_DEBUG_ASSERT(rc == APR_SUCCESS);
> }
> + return;
> }
> else if (cs->pub.state == CONN_STATE_SUSPENDED) {
> apr_atomic_inc32(&suspended_count);
>
May as well clear sbh in other places where process_socket() returns,
right?
(The notify_suspend()/notify_resume() which corresponds to clearing/setting
sbh is just a sketch for a new hook; it seems that, the thread
notifications should match clearing/setting/sbh inside process_socket().)
Index: server/mpm/event/event.c
===================================================================
--- server/mpm/event/event.c (revision 1545642)
+++ server/mpm/event/event.c (working copy)
@@ -976,6 +976,7 @@
else {
c = cs->c;
c->sbh = sbh;
+ notify_resume(cs);
c->current_thread = thd;
}
@@ -1028,6 +1029,8 @@
* event thread poll for writeability.
*/
cs->expiration_time = ap_server_conf->timeout + apr_time_now();
+ c->sbh = NULL;
+ notify_suspend(cs);
apr_thread_mutex_lock(timeout_mutex);
TO_QUEUE_APPEND(write_completion_q, cs);
cs->pfd.reqevents = (
@@ -1052,8 +1055,11 @@
}
if (cs->pub.state == CONN_STATE_LINGER) {
- if (!start_lingering_close_blocking(cs))
+ if (!start_lingering_close_blocking(cs)) {
+ c->sbh = NULL;
+ notify_suspend(cs);
return;
+ }
}
else if (cs->pub.state == CONN_STATE_CHECK_REQUEST_LINE_READABLE) {
/* It greatly simplifies the logic to use a single timeout value
here
@@ -1067,6 +1073,7 @@
cs->expiration_time = ap_server_conf->keep_alive_timeout +
apr_time_now();
c->sbh = NULL;
+ notify_suspend(cs);
apr_thread_mutex_lock(timeout_mutex);
TO_QUEUE_APPEND(keepalive_q, cs);
@@ -1092,6 +1099,7 @@
* or timeout.
*/
c->sbh = NULL;
+ notify_suspend(cs);
return;
}
> Modified: httpd/httpd/trunk/server/mpm/eventopt/eventopt.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/eventopt/eventopt.c?rev=1538490&r1=1538489&r2=1538490&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/server/mpm/eventopt/eventopt.c (original)
> +++ httpd/httpd/trunk/server/mpm/eventopt/eventopt.c Mon Nov 4 01:37:31
> 2013
> @@ -1124,7 +1124,7 @@ read_request:
> */
> cs->expiration_time = ap_server_conf->keep_alive_timeout +
> apr_time_now();
> -
> + c->sbh = NULL;
> /* Add work to pollset. */
> v = ap_equeue_writer_value(eq);
> v->timeout_type = TIMEOUT_KEEPALIVE;
> @@ -1133,6 +1133,7 @@ read_request:
> v->tag = "process_socket(keepalive)";
> ap_equeue_writer_onward(eq);
> apr_pollset_wakeup(event_pollset);
> + return;
> }
> else if (cs->pub.state == CONN_STATE_SUSPENDED) {
> apr_atomic_inc32(&suspended_count);
>
>
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
