From: Jim Jagielski <jim@apache.org>
Date: Tue, 26 Feb 2008 19:51:29 +0000 (+0000)
Subject: Merge r630335, r630348, r630350 from trunk:
X-Git-Tag: 2.2.9~343
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=236049b8b92f5861a1c4d1e0e2932ffe2a7bda47;p=thirdparty%2Fapache%2Fhttpd.git

Merge r630335, r630348, r630350 from trunk:

Worker MPM: fix race condition
PR44402: reported and fixed by Basant Kumar Kukreja


* Second part of fix for PR 44402:

  - Fix the same race condition in event MPM.
  - Slightly optimize code in worker MPM by removing the need for an additional
    dereference operation.
  - Do some word smithing on the CHANGES entry.

PR: 44402
Submitted by: Basant Kumar Kukreja <basant.kukreja sun.com>
Reviewed by: rpluem


* Add hint to PR in comment. No functional change.

Reviewed by: jim


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@631362 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index bfa2a9c6f93..d1fd278f809 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.9
 
+  *) Worker / Event MPM: Fix race condition in pool recycling that leads to
+     segmentation faults under load.  PR 44402
+     [Basant Kumar Kukreja <basant.kukreja sun.com>]
+
   *) mod_proxy_ftp: Fix base for directory listings.
      PR 27834 [Nick Kew]
 
diff --git a/STATUS b/STATUS
index 2880e6baae0..7166a445bfd 100644
--- a/STATUS
+++ b/STATUS
@@ -81,13 +81,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
- * Worker and Event MPMs: Fix race condition in pool recycling
-   PR 44402
-   http://svn.apache.org/viewvc?rev=630335&view=rev
-   http://svn.apache.org/viewvc?rev=630348&view=rev
-   http://svn.apache.org/viewvc?rev=630350&view=rev
-   +1: niq, rpluem, jim
-
  * Don't add bogus duplicate Content-Language entries to generated output
    PR 11035
    http://svn.apache.org/viewvc?rev=611475&view=rev
diff --git a/server/mpm/experimental/event/fdqueue.c b/server/mpm/experimental/event/fdqueue.c
index c0ef9bbbb1f..5a1baa94d86 100644
--- a/server/mpm/experimental/event/fdqueue.c
+++ b/server/mpm/experimental/event/fdqueue.c
@@ -194,10 +194,16 @@ void ap_push_pool(fd_queue_info_t * queue_info,
                                                           (*new_recycle));
         new_recycle->pool = pool_to_recycle;
         for (;;) {
-            new_recycle->next = queue_info->recycled_pools;
+            /*
+             * Save queue_info->recycled_pool in local variable next because
+             * new_recycle->next can be changed after apr_atomic_casptr
+             * function call. For gory details see PR 44402.
+             */
+            struct recycled_pool *next = queue_info->recycled_pools;
+            new_recycle->next = next;
             if (apr_atomic_casptr
                 ((volatile void **) &(queue_info->recycled_pools),
-                 new_recycle, new_recycle->next) == new_recycle->next) {
+                 new_recycle, next) == next) {
                 break;
             }
         }
diff --git a/server/mpm/worker/fdqueue.c b/server/mpm/worker/fdqueue.c
index 8be7c9fa2da..8a75d24d1ed 100644
--- a/server/mpm/worker/fdqueue.c
+++ b/server/mpm/worker/fdqueue.c
@@ -94,10 +94,14 @@ apr_status_t ap_queue_info_set_idle(fd_queue_info_t *queue_info,
                                                          sizeof(*new_recycle));
         new_recycle->pool = pool_to_recycle;
         for (;;) {
-            new_recycle->next = queue_info->recycled_pools;
+            /* Save queue_info->recycled_pool in local variable next because
+             * new_recycle->next can be changed after apr_atomic_casptr
+             * function call. For gory details see PR 44402.
+             */
+            struct recycled_pool *next = queue_info->recycled_pools;
+            new_recycle->next = next;
             if (apr_atomic_casptr((volatile void**)&(queue_info->recycled_pools),
-                                  new_recycle, new_recycle->next) ==
-                new_recycle->next) {
+                                  new_recycle, next) == next) {
                 break;
             }
         }