winnt child.c

Bill Stoddard Wed, 25 Aug 2004 08:52:59 -0700

I have some questions about this patch (just now looking at porting it to 2.0):

[EMAIL PROTECTED] wrote:

ake         2004/05/03 15:22:50
Modified: . CHANGES server/mpm/winnt child.c Log: Prevent Win32 pool corruption at startup 1.36 +17 -2 httpd-2.0/server/mpm/winnt/child.c Index: child.c =================================================================== RCS file: /home/cvs/httpd-2.0/server/mpm/winnt/child.c,v retrieving revision 1.35 retrieving revision 1.36 diff -u -r1.35 -r1.36 --- child.c 15 Mar 2004 23:08:41 -0000 1.35 +++ child.c 3 May 2004 22:22:50 -0000 1.36 @@ -58,7 +58,7 @@ static unsigned int g_blocked_threads = 0; static HANDLE max_requests_per_child_event; - +static apr_thread_mutex_t *child_lock; static apr_thread_mutex_t *qlock; static PCOMP_CONTEXT qhead = NULL; static PCOMP_CONTEXT qtail = NULL; @@ -145,6 +145,7 @@ */ apr_allocator_t *allocator; + apr_thread_mutex_lock(child_lock); context = (PCOMP_CONTEXT) apr_pcalloc(pchild, sizeof(COMP_CONTEXT)); context->Overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); @@ -152,6 +153,8 @@ /* 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."); + + apr_thread_mutex_unlock(child_lock); return NULL; } @@ -163,6 +166,8 @@ 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); + + apr_thread_mutex_unlock(child_lock); return NULL; } apr_allocator_owner_set(allocator, context->ptrans); @@ -171,6 +176,8 @@ context->accept_socket = INVALID_SOCKET; context->ba = apr_bucket_alloc_create(pchild); apr_atomic_inc32(&num_completion_contexts); + + apr_thread_mutex_unlock(child_lock); break; } } else {


Here is the actual code from this section of the patch with my comments:

                /* 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...
                 */
                apr_allocator_t *allocator;

===> lock here  apr_thread_mutex_lock(child_lock);
===> this presumes the calls to apr_pcalloc, createevent, et need protection. I think
===> they don't need protection

                context = (PCOMP_CONTEXT) apr_pcalloc(pchild, sizeof(COMP_CONTEXT));

                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.");

                    apr_thread_mutex_unlock(child_lock);
                    return NULL;
                }

                /* Create the tranaction pool */
==> needs protection?   apr_allocator_create(&allocator);
==> needs protection?   apr_allocator_max_free_set(allocator, ap_max_mem_free);
==> can we move the lock call here???
                rv = apr_pool_create_ex(&context->ptrans, pchild, NULL, allocator);
==> and add unlock call here?
                if (rv != 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);
==> and delete this?
                    apr_thread_mutex_unlock(child_lock);
                    return NULL;
                }
                apr_allocator_owner_set(allocator, context->ptrans);
                apr_pool_tag(context->ptrans, "transaction");

                context->accept_socket = INVALID_SOCKET;
                context->ba = apr_bucket_alloc_create(pchild);
                apr_atomic_inc32(&num_completion_contexts);
==> and delete this?
                apr_thread_mutex_unlock(child_lock);
                break;
            }

<snip>

  +            apr_thread_mutex_lock(child_lock);
               score_idx = apr_pcalloc(pchild, sizeof(int));
               *score_idx = i;
               apr_hash_set(ht, &child_handles[i], sizeof(HANDLE), score_idx);
  +            apr_thread_mutex_unlock(child_lock);
           }

This looks okay as child_handles and ht can be access my multiple threads at once, 
thus needs protection.

Comments?

Bill

Re: cvs commit: httpd-2.0/server/mpm/winnt child.c

Reply via email to