Merge r1741310, r1741461 from trunk:

  Ensure http2 follows http in the meaning of
  status WRITE (meaning 'in the request processing
  phase' even if still consuming the request body,
  not literally in a 'now writing' state).

  Ensure a number of MPMs and the h2 connection io
  no longer clobber the request status line during
  state-only changes.  While at it, clean up some
  very ugly formatting and unnecessary decoration,
  and avoid the wordy _from_conn() flavor when we
  are not passing a connection_rec.

  Ensure the useragent_ip is only used in the case
  where it has been initialized, fall back on the
  connection's remote_ip if the status is accidently
  updated from an uninitialized request_rec.

Revert to 2.4.1 behavior and null the request and vhost as appropriate.
PR: 59333
Submitted by: wrowe
Reviewed by: wrowe, ylavic, rpluem

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

diff --git a/CHANGES b/CHANGES
index 60e73706d89c0b3d5c2d8b1ce54f648da718c2be..97f1ae47c814f92e40910c28c806d826a8e6b765 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 
 Changes with Apache 2.4.21
 
+  *) Scoreboard: Fix a regression in 2.4.20 that causes wrong request data
+     to be displayed on the status page. PR 59333. [Yann Ylavic, William Rowe]
+
   *) mod_http2: fixed a bug that caused mod_proxy_http2 to be called for window
      updates on requests it had already reported done. Added synchronization
      on early connection/stream close that lets ongoing requests safely drain
@@ -11,12 +14,12 @@ Changes with Apache 2.4.21
   *) mod_http2: scoreboard updates that summarize the h2 session (and replace
      the last request information) will only happen when the session is idle or 
      in shutdown/done phase. [Stefan Eissing]
-     
+
   *) mod_http2: new "bucket beam" technology to transport buckets across
      threads without buffer copy. Delaying response start until flush or
      enough body data has been accumulated. Overall significanlty smaller
      memory footprint. [Stefan Eissing]
-     
+
   *) core: New CGIVar directive can configure REQUEST_URI to represent the
      current URI being processed instead of always the original request.
      [Jeff Trawick]
@@ -27,10 +30,10 @@ Changes with Apache 2.4.21
   *) mod_http2: r->protocol changed to "HTTP/2.0" (was "HTTP/2") as this will
      give expected syntax in CGI's SERVER_PROTOCOL is more compatible with
      existing major/minor handling. Fixes PR 59313.
-  
+
   *) mod_http2: disabling mmap for file buckets transport due to segmenation
      faults when files change on the fly.
-     
+
 Changes with Apache 2.4.20
 
   *) core: Do not read .htaccess if AllowOverride and AllowOverrideList
@@ -42,7 +45,7 @@ Changes with Apache 2.4.20
 
   *) core/util_script: relax alphanumeric filter of enviroment variable names
      on Windows to allow '(' and ')' for passing PROGRAMFILES(X86) et.al.
-     unadulterated in 64 bit versions of Windows. PR 46751.  
+     unadulterated in 64 bit versions of Windows. PR 46751.
      [John <john leineweb de>]
 
   *) mod_http2: incrementing keepalives on each request started so that logging
@@ -56,7 +59,7 @@ Changes with Apache 2.4.20
 
   *) mod_http2: fix for missing score board updates on request count, fix for
      memory leak on slave connection reuse. [Stefan Eissing]
-     
+
   *) mod_http2: Fix build on Windows from dsp files.
      [Stefan Eissing] 
 

diff --git a/STATUS b/STATUS
index f7e4eb8e54f4bebe01716fcffbd99949f5d5ab35..0ea581789053748bde3bd193ef8e75033a29b0be 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -114,15 +114,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) Correct scoreboard behaviors in multiple MPMs and mod_http2 to avoid
-     clobbering the most recent request line at unexpect times.  Corrects
-     a notable regression in 2.4.20 and in some earlier edge cases.
-     PR 59333
-     See r1741310 for more precise patch details
-     trunk patch: http://svn.apache.org/r1741310
-                  http://svn.apache.org/r1741461
-     2.4.x patch: https://raw.githubusercontent.com/wrowe/patches/master/backport-httpd-2.4-r1741310-1741461.patch
-     +1: wrowe, ylavic, rpluem
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
@@ -133,7 +124,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
                     http://svn.apache.org/r1740119
      2.4.x: trunk works modulo CHANGES
      +1: icing
-     
+
   *) mod_proxy_hcheck: Dynamic reverse proxy backend health check module
      Trunk version of patch: <various>
      Backport version for 2.4.x of patch:

diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c
index badee774240dee27606b45015b96d064e43482c5..2e0a5f2d690241a56e39d9453c6cd96a75077d6a 100644 (file)
--- a/modules/http2/h2_conn_io.c
+++ b/modules/http2/h2_conn_io.c
@@ -191,7 +191,7 @@ static apr_status_t pass_out(apr_bucket_brigade *bb, void *ctx)
         return APR_SUCCESS;
     }
     
-    ap_update_child_status_from_conn(c->sbh, SERVER_BUSY_WRITE, c);
+    ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, NULL);
     apr_brigade_length(bb, 0, &bblen);
     h2_conn_io_bb_log(c, 0, APLOG_TRACE2, "master conn pass", bb);
     status = ap_pass_brigade(c->output_filters, bb);

diff --git a/modules/http2/h2_task.c b/modules/http2/h2_task.c
index fe0cca25849eac09b82e6c550325b5ae7ad9c30c..93bb479ea6afed7f29de155b5bd31ad7980a8b65 100644 (file)
--- a/modules/http2/h2_task.c
+++ b/modules/http2/h2_task.c
@@ -682,7 +682,7 @@ static apr_status_t h2_task_process_request(h2_task *task, conn_rec *c)
                   "h2_task(%s): create request_rec", task->id);
     r = h2_request_create_rec(req, c);
     if (r && (r->status == HTTP_OK)) {
-        ap_update_child_status(c->sbh, SERVER_BUSY_READ, r);
+        ap_update_child_status(c->sbh, SERVER_BUSY_WRITE, r);
         
         if (cs) {
             cs->state = CONN_STATE_HANDLER;

diff --git a/server/core.c b/server/core.c
index c5900709728bf21079cf075480fa5bc6ee2d2295..412ba34e59c355958445bf4b1f58679baf3b3cc7 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -4877,7 +4877,7 @@ static conn_rec *core_create_conn(apr_pool_t *ptrans, server_rec *server,
     conn_rec *c = (conn_rec *) apr_pcalloc(ptrans, sizeof(conn_rec));
 
     c->sbh = sbh;
-    (void)ap_update_child_status(c->sbh, SERVER_BUSY_READ, (request_rec *)NULL);
+    ap_update_child_status(c->sbh, SERVER_BUSY_READ, NULL);
 
     /* Got a connection structure, so initialize what fields we can
      * (the rest are zeroed out by pcalloc).

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index 2418471fd05156fd1938de1231e7e8da55c096d2..9dd91cde2edc289f7c49ea4149950d9e2eb0d0ed 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -1114,7 +1114,7 @@ read_request:
     if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
         ap_filter_t *output_filter = c->output_filters;
         apr_status_t rv;
-        ap_update_child_status_from_conn(sbh, SERVER_BUSY_WRITE, c);
+        ap_update_child_status(sbh, SERVER_BUSY_WRITE, NULL);
         while (output_filter->next != NULL) {
             output_filter = output_filter->next;
         }
@@ -1910,7 +1910,8 @@ static void *APR_THREAD_FUNC worker_thread(apr_thread_t * thd, void *dummy)
         }
 
         ap_update_child_status_from_indexes(process_slot, thread_slot,
-                                            dying ? SERVER_GRACEFUL : SERVER_READY, NULL);
+                                            dying ? SERVER_GRACEFUL
+                                                  : SERVER_READY, NULL);
       worker_pop:
         if (workers_may_exit) {
             break;
@@ -1965,9 +1966,8 @@ static void *APR_THREAD_FUNC worker_thread(apr_thread_t * thd, void *dummy)
     }
 
     ap_update_child_status_from_indexes(process_slot, thread_slot,
-                                        dying ? SERVER_DEAD :
-                                        SERVER_GRACEFUL,
-                                        (request_rec *) NULL);
+                                        dying ? SERVER_DEAD
+                                              : SERVER_GRACEFUL, NULL);
 
     apr_thread_exit(thd, APR_SUCCESS);
     return NULL;
@@ -2728,8 +2728,7 @@ static void server_main_loop(int remaining_children_to_start, int num_buckets)
 
                 for (i = 0; i < threads_per_child; i++)
                     ap_update_child_status_from_indexes(child_slot, i,
-                                                        SERVER_DEAD,
-                                                        (request_rec *) NULL);
+                                                        SERVER_DEAD, NULL);
 
                 event_note_child_killed(child_slot, 0, 0);
                 ps = &ap_scoreboard_image->parent[child_slot];

diff --git a/server/mpm/winnt/child.c b/server/mpm/winnt/child.c
index e4d1c150a1865c1ed7d091bae9ca45e5641da429..dee35df5b54d7283b39829feeca1977e6eae3721 100644 (file)
--- a/server/mpm/winnt/child.c
+++ b/server/mpm/winnt/child.c
@@ -894,8 +894,7 @@ static DWORD __stdcall worker_main(void *thread_num_val)
         }
     }
 
-    ap_update_child_status_from_indexes(0, thread_num, SERVER_DEAD,
-                                        (request_rec *) NULL);
+    ap_update_child_status_from_indexes(0, thread_num, SERVER_DEAD, NULL);
 
     return 0;
 }
@@ -1314,13 +1313,13 @@ void child_main(apr_pool_t *pconf, DWORD parent_pid)
                      threads_created);
     }
     for (i = 0; i < threads_created; i++) {
-        int *score_idx;
+        int *idx;
         TerminateThread(child_handles[i], 1);
         CloseHandle(child_handles[i]);
         /* Reset the scoreboard entry for the thread we just whacked */
-        score_idx = apr_hash_get(ht, &child_handles[i], sizeof(HANDLE));
-        if (score_idx) {
-            ap_update_child_status_from_indexes(0, *score_idx, SERVER_DEAD, NULL);
+        idx = apr_hash_get(ht, &child_handles[i], sizeof(HANDLE));
+        if (idx) {
+            ap_update_child_status_from_indexes(0, *idx, SERVER_DEAD, NULL);
         }
     }
     ap_log_error(APLOG_MARK, APLOG_NOTICE, APR_SUCCESS, ap_server_conf, APLOGNO(00364)

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index 24aa46bd20dfb3d1628f540537cfbff2098ed0cd..01a86cb3a5db6b925203c20611fe9470f558b856 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -928,7 +928,8 @@ static void * APR_THREAD_FUNC worker_thread(apr_thread_t *thd, void * dummy)
     ap_scoreboard_image->servers[process_slot][thread_slot].pid = ap_my_pid;
     ap_scoreboard_image->servers[process_slot][thread_slot].tid = apr_os_thread_current();
     ap_scoreboard_image->servers[process_slot][thread_slot].generation = retained->my_generation;
-    ap_update_child_status_from_indexes(process_slot, thread_slot, SERVER_STARTING, NULL);
+    ap_update_child_status_from_indexes(process_slot, thread_slot,
+                                        SERVER_STARTING, NULL);
 
 #ifdef HAVE_PTHREAD_KILL
     unblock_signal(WORKER_SIGNAL);
@@ -949,7 +950,8 @@ static void * APR_THREAD_FUNC worker_thread(apr_thread_t *thd, void * dummy)
             is_idle = 1;
         }
 
-        ap_update_child_status_from_indexes(process_slot, thread_slot, SERVER_READY, NULL);
+        ap_update_child_status_from_indexes(process_slot, thread_slot,
+                                            SERVER_READY, NULL);
 worker_pop:
         if (workers_may_exit) {
             break;
@@ -995,7 +997,8 @@ worker_pop:
     }
 
     ap_update_child_status_from_indexes(process_slot, thread_slot,
-        (dying) ? SERVER_DEAD : SERVER_GRACEFUL, (request_rec *) NULL);
+                                        dying ? SERVER_DEAD
+                                              : SERVER_GRACEFUL, NULL);
 
     apr_thread_exit(thd, APR_SUCCESS);
     return NULL;
@@ -1730,8 +1733,8 @@ static void server_main_loop(int remaining_children_to_start, int num_buckets)
                 process_score *ps;
 
                 for (i = 0; i < threads_per_child; i++)
-                    ap_update_child_status_from_indexes(child_slot, i, SERVER_DEAD,
-                                                        (request_rec *) NULL);
+                    ap_update_child_status_from_indexes(child_slot, i,
+                                                        SERVER_DEAD, NULL);
 
                 worker_note_child_killed(child_slot, 0, 0);
                 ps = &ap_scoreboard_image->parent[child_slot];

diff --git a/server/scoreboard.c b/server/scoreboard.c
index 007a5e8837e7e888319ccf4da26f2fecfb69bead..8e3403f73595a65ecaade96036bc1d02ec852fd7 100644 (file)
--- a/server/scoreboard.c
+++ b/server/scoreboard.c
@@ -500,8 +500,11 @@ static int update_child_status_internal(int child_num,
         else if (r) {
             copy_request(ws->request, sizeof(ws->request), r);
         }
+        else if (c) {
+            ws->request[0]='\0';
+        }
 
-        if (r) {
+        if (r && r->useragent_ip) {
             if (!(val = ap_get_useragent_host(r, REMOTE_NOLOOKUP, NULL)))
                 apr_cpystrn(ws->client, r->useragent_ip, sizeof(ws->client));
             else
@@ -524,6 +527,9 @@ static int update_child_status_internal(int child_num,
                 apr_cpystrn(ws->vhost, s->server_hostname, sizeof(ws->vhost));
             }
         }
+        else if (c) {
+            ws->vhost[0]='\0';
+        }
 
         if (c) {
             val = ap_get_protocol(c);