[PATCH] Avoid busy wait in winnt MPM when all workers are busy

[Igor Nazarenko]

The only modification is in
./server/mpm/winnt/child.c

That file hasn't changed between 2.0 and 2.1

This is the first time I'm submitting a patch so please tell me if I'm doing something wrong.

Igor

_________________________________________________________________
The new MSN 8: smart spam protection and 2 months FREE* http://join.msn.com/?page=features/junkmail
--- child.c.old Mon Oct 14 07:54:45 2002
+++ child.c Wed Jan 8 09:55:12 2003
@@ -104,12 +104,14 @@


static apr_thread_mutex_t *qlock;
+static HANDLE qwait_event = NULL;
static PCOMP_CONTEXT qhead = NULL;
static PCOMP_CONTEXT qtail = NULL;
static int num_completion_contexts = 0;
static HANDLE ThreadDispatchIOCP = NULL;


+
AP_DECLARE(void) mpm_recycle_completion_context(PCOMP_CONTEXT context)
{
/* Recycle the completion context.
@@ -124,10 +126,12 @@
context->next = NULL;
ResetEvent(context->Overlapped.hEvent);
apr_thread_mutex_lock(qlock);
- if (qtail)
+ if (qtail) {
qtail->next = context;
- else
+ } else {
qhead = context;
+ SetEvent(qwait_event);
+ }
qtail = context;
apr_thread_mutex_unlock(qlock);
}
@@ -138,57 +142,82 @@
apr_status_t rv;
PCOMP_CONTEXT context = NULL;

- /* Grab a context off the queue */
- apr_thread_mutex_lock(qlock);
- if (qhead) {
- context = qhead;
- qhead = qhead->next;
- if (!qhead)
- qtail = NULL;
- }
- apr_thread_mutex_unlock(qlock);
-
- /* If we failed to grab a context off the queue, alloc one out of
- * the child pool. There may be up to ap_threads_per_child contexts
- * in the system at once.
- */
- if (!context) {
- if (num_completion_contexts >= ap_threads_per_child) {
- static int reported = 0;
- if (!reported) {
- ap_log_error(APLOG_MARK, APLOG_WARNING, 0, ap_server_conf,
- "Server ran out of threads to serve requests. Consider "
- "raising the ThreadsPerChild setting");
- reported = 1;
- }
- return NULL;
+ while (1) {
+ /* Grab a context off the queue */
+ apr_thread_mutex_lock(qlock);
+ if (qhead) {
+ context = qhead;
+ qhead = qhead->next;
+ if (!qhead)
+ qtail = NULL;
+ } else {
+ ResetEvent(qwait_event);
}
- /* Note:
- * Multiple failures in the next two steps will cause the pchild pool
- * to 'leak' storage. I don't think this is worth fixing...
- */
- context = (PCOMP_CONTEXT) apr_pcalloc(pchild, sizeof(COMP_CONTEXT));
+ apr_thread_mutex_unlock(qlock);

- context->Overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
- if (context->Overlapped.hEvent == NULL) {
- /* Hopefully this is a temporary condition ... */
- ap_log_error(APLOG_MARK,APLOG_WARNING, apr_get_os_error(), ap_server_conf,
- "mpm_get_completion_context: CreateEvent failed.");
- return NULL;
- }
+ if (!context) {
+ /* We failed to grab a context off the queue, consider allocating a
+ * new one out of the child pool. There may be up to ap_threads_per_child
+ * contexts in the system at once.
+ * Note: this function assumes that only one thread may call it,
+ * so the access to num_completion_contexts variable does not
+ * have to be protected.
+ */
+ if (num_completion_contexts >= ap_threads_per_child) {
+ /* All workers are busy, need to wait for one */
+ static int reported = 0;
+ if (!reported) {
+ ap_log_error(APLOG_MARK, APLOG_WARNING, 0, ap_server_conf,
+ "Server ran out of threads to serve requests. Consider "
+ "raising the ThreadsPerChild setting");
+ reported = 1;
+ }

- /* Create the tranaction pool */
- if ((rv = apr_pool_create(&context->ptrans, pchild)) != APR_SUCCESS) {
- ap_log_error(APLOG_MARK,APLOG_WARNING, rv, ap_server_conf,
- "mpm_get_completion_context: Failed to create the transaction pool.");
- CloseHandle(context->Overlapped.hEvent);
- return NULL;
- }
- apr_pool_tag(context->ptrans, "ptrans");
+ /* Wait for a worker to free a context. Once per second, give
+ * the caller a chance to check for shutdown. If the wait
+ * succeeds, get the context off the queue. It must be available,
+ * since there's only one consumer.
+ */
+ rv = WaitForSingleObject(qwait_event, 1000);
+ if (rv == WAIT_OBJECT_0)
+ continue;
+ else /* Hopefully, WAIT_TIMEOUT */
+ return NULL;
+ } else {
+ /* Allocate another context.
+ * Note:
+ * Multiple failures in the next two steps will cause the pchild pool
+ * to 'leak' storage. I don't think this is worth fixing...
+ */
+ context = (PCOMP_CONTEXT) apr_pcalloc(pchild, sizeof(COMP_CONTEXT));

- context->accept_socket = INVALID_SOCKET;
- context->ba = apr_bucket_alloc_create(pchild);
- apr_atomic_inc(&num_completion_contexts);
+ context->Overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
+ if (context->Overlapped.hEvent == NULL) {
+ /* Hopefully this is a temporary condition ... */
+ ap_log_error(APLOG_MARK,APLOG_WARNING, apr_get_os_error(), ap_server_conf,
+ "mpm_get_completion_context: CreateEvent failed.");
+ return NULL;
+ }
+
+ /* Create the tranaction pool */
+ if ((rv = apr_pool_create(&context->ptrans, pchild)) != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK,APLOG_WARNING, rv, ap_server_conf,
+ "mpm_get_completion_context: Failed to create the transaction pool.");
+ CloseHandle(context->Overlapped.hEvent);
+ return NULL;
+ }
+ apr_pool_tag(context->ptrans, "ptrans");
+
+ context->accept_socket = INVALID_SOCKET;
+ context->ba = apr_bucket_alloc_create(pchild);
+ num_completion_contexts++;
+
+ break;
+ }
+ } else {
+ /* Got a context from the queue */
+ break;
+ }
}

return context;
@@ -823,6 +852,12 @@
0,
0); /* CONCURRENT ACTIVE THREADS */
apr_thread_mutex_create(&qlock, APR_THREAD_MUTEX_DEFAULT, pchild);
+ qwait_event = CreateEvent(NULL, TRUE, FALSE, NULL);
+ if (!qwait_event) {
+ ap_log_error(APLOG_MARK, APLOG_CRIT, apr_get_os_error(), ap_server_conf,
+ "Child %d: Failed to create a qwait event.", my_pid);
+ exit(APEXIT_CHILDINIT);
+ }
}

/*
@@ -1018,8 +1053,10 @@

CloseHandle(allowed_globals.jobsemaphore);
apr_thread_mutex_destroy(allowed_globals.jobmutex);
- if (osver.dwPlatformId != VER_PLATFORM_WIN32_WINDOWS)
+ if (osver.dwPlatformId != VER_PLATFORM_WIN32_WINDOWS) {
apr_thread_mutex_destroy(qlock);
+ CloseHandle(qwait_event);
+ }

apr_pool_destroy(pchild);
CloseHandle(exit_event);