You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2014/12/02 09:09:54 UTC
Re: svn commit: r1639614 - /httpd/httpd/trunk/server/mpm/event/event.c
On Fri, Nov 14, 2014 at 1:04 PM, <co...@apache.org> wrote:
> Author: covener
> Date: Fri Nov 14 12:04:46 2014
> New Revision: 1639614
>
> URL: http://svn.apache.org/r1639614
> Log:
> don't call notify_suspend() in a worker thread after
> start_lingering_close_common may have put the socket back
> into the pollset.
>
> If it becomes readable too quickly, cs can be
> free'ed or accessed concurrently.
>
> Modified:
> httpd/httpd/trunk/server/mpm/event/event.c
>
[]
> +static int start_lingering_close_common(event_conn_state_t *cs, int in_worker)
> {
> apr_status_t rv;
> struct timeout_queue *q;
> @@ -849,6 +861,9 @@ static int start_lingering_close_common(
> cs->pub.sense == CONN_SENSE_WANT_WRITE ? APR_POLLOUT :
> APR_POLLIN) | APR_POLLHUP | APR_POLLERR;
> cs->pub.sense = CONN_SENSE_DEFAULT;
> + if (in_worker) {
> + notify_suspend(cs);
> + }
Can't we put this outside the critical section?
(should the hook take non negligible time).
> rv = apr_pollset_add(event_pollset, &cs->pfd);
> apr_thread_mutex_unlock(timeout_mutex);
> if (rv != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rv)) {
Eg.
Index: server/mpm/event/event.c
===================================================================
--- server/mpm/event/event.c (revision 1642684)
+++ server/mpm/event/event.c (working copy)
@@ -859,16 +859,16 @@ static int start_lingering_close_common(event_conn
cs->pub.state = CONN_STATE_LINGER_NORMAL;
}
apr_atomic_inc32(&lingering_count);
+ cs->c->sbh = NULL;
+ if (in_worker) {
+ notify_suspend(cs);
+ }
apr_thread_mutex_lock(timeout_mutex);
- cs->c->sbh = NULL;
TO_QUEUE_APPEND(*q, cs);
cs->pfd.reqevents = (
cs->pub.sense == CONN_SENSE_WANT_WRITE ? APR_POLLOUT :
APR_POLLIN) | APR_POLLHUP | APR_POLLERR;
cs->pub.sense = CONN_SENSE_DEFAULT;
- if (in_worker) {
- notify_suspend(cs);
- }
rv = apr_pollset_add(event_pollset, &cs->pfd);
apr_thread_mutex_unlock(timeout_mutex);
if (rv != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rv)) {
--
Re: svn commit: r1639614 - /httpd/httpd/trunk/server/mpm/event/event.c
Posted by Eric Covener <co...@gmail.com>.
> Index: server/mpm/event/event.c
> ===================================================================
> --- server/mpm/event/event.c (revision 1642684)
> +++ server/mpm/event/event.c (working copy)
> @@ -859,16 +859,16 @@ static int start_lingering_close_common(event_conn
> cs->pub.state = CONN_STATE_LINGER_NORMAL;
> }
> apr_atomic_inc32(&lingering_count);
> + cs->c->sbh = NULL;
> + if (in_worker) {
> + notify_suspend(cs);
> + }
> apr_thread_mutex_lock(timeout_mutex);
> - cs->c->sbh = NULL;
> TO_QUEUE_APPEND(*q, cs);
> cs->pfd.reqevents = (
> cs->pub.sense == CONN_SENSE_WANT_WRITE ? APR_POLLOUT :
> APR_POLLIN) | APR_POLLHUP | APR_POLLERR;
> cs->pub.sense = CONN_SENSE_DEFAULT;
> - if (in_worker) {
> - notify_suspend(cs);
> - }
> rv = apr_pollset_add(event_pollset, &cs->pfd);
> apr_thread_mutex_unlock(timeout_mutex);
> if (rv != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rv)) {
+1