On Fri, Nov 14, 2014 at 1:04 PM, <cove...@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)) { --