svn commit: r1892980 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/revert_r887244_and_r887245.txt modules/metadata/mod_unique_id.c

[yl...@apache.org]

Author: ylavic
Date: Mon Sep  6 10:17:04 2021
New Revision: 1892980

URL: http://svn.apache.org/viewvc?rev=1892980&view=rev
Log:
Merge r1892914 from trunk:

Revert r1887244 and r1887245 which causes issues on Windows

Submitted by: jailletc36
Reviewed by: jailletc36, icing, jorton

Added:
    httpd/httpd/branches/2.4.x/changes-entries/revert_r887244_and_r887245.txt
      - copied unchanged from r1892914, httpd/httpd/trunk/changes-entries/revert_r887244_and_r887245.txt
Modified:
    httpd/httpd/branches/2.4.x/   (props changed)
    httpd/httpd/branches/2.4.x/modules/metadata/mod_unique_id.c

Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
  Merged /httpd/httpd/trunk:r1892914

Modified: httpd/httpd/branches/2.4.x/modules/metadata/mod_unique_id.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/metadata/mod_unique_id.c?rev=1892980&r1=1892979&r2=1892980&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/metadata/mod_unique_id.c (original)
+++ httpd/httpd/branches/2.4.x/modules/metadata/mod_unique_id.c Mon Sep  6 10:17:04 2021
@@ -42,6 +42,12 @@ typedef struct {
 
 /* We are using thread_index (the index into the scoreboard), because we
  * cannot guarantee the thread_id will be an integer.
+ *
+ * This code looks like it won't give a unique ID with the new thread logic.
+ * It will.  The reason is, we don't increment the counter in a thread_safe
+ * manner.  Because the thread_index is also in the unique ID now, this does
+ * not matter.  In order for the id to not be unique, the same thread would
+ * have to get the same counter twice in the same second.
  */
 
 /* Comments:
@@ -63,7 +69,7 @@ typedef struct {
  * The stamp and counter are used to distinguish all hits for a
  * particular root.  The stamp is updated using r->request_time,
  * saving cpu cycles.  The counter is never reset, and is used to
- * permit up to 64k requests in a single second by a single thread.
+ * permit up to 64k requests in a single second by a single child.
  *
  * The 144-bits of unique_id_rec are encoded using the alphabet
  * [A-Za-z0-9@-], resulting in 24 bytes of printable characters.  That is then
@@ -110,6 +116,12 @@ typedef struct {
  * htonl/ntohl. Well, this shouldn't be a problem till year 2106.
  */
 
+/*
+ * XXX: We should have a per-thread counter and not use cur_unique_id.counter
+ * XXX: in all threads, because this is bad for performance on multi-processor
+ * XXX: systems: Writing to the same address from several CPUs causes cache
+ * XXX: thrashing.
+ */
 static unique_id_rec cur_unique_id;
 
 /*
@@ -170,13 +182,11 @@ static const char uuencoder[64] = {
     '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '-', '_',
 };
 
-#define THREADED_COUNTER "unique_id_counter"
-
 static const char *gen_unique_id(const request_rec *r)
 {
     char *str;
     /*
-     * Buffer padded with two final bytes, used to copy the unique_id_rec
+     * Buffer padded with two final bytes, used to copy the unique_id_red
      * structure without the internal paddings that it could have.
      */
     unique_id_rec new_unique_id;
@@ -188,35 +198,8 @@ static const char *gen_unique_id(const r
     unsigned short counter;
     int i,j,k;
 
-#if APR_HAS_THREADS
-    apr_status_t rv;
-    unsigned short *pcounter;
-    apr_thread_t *thread = r->connection->current_thread;
-    
-    rv = apr_thread_data_get((void **)&pcounter, THREADED_COUNTER, thread);
-    if (rv == APR_SUCCESS && pcounter) {
-        counter = *pcounter;
-    }
-    else
-#endif
-    {
-        counter = cur_unique_id.counter;
-    }
-
     memcpy(&new_unique_id.root, &cur_unique_id.root, ROOT_SIZE);
-    new_unique_id.counter = htons(counter++);
-#if APR_HAS_THREADS
-    if (!pcounter) {
-        pcounter = apr_palloc(apr_thread_pool_get(thread), sizeof(*pcounter));
-    }
-    
-    *pcounter = counter;
-    rv = apr_thread_data_set(pcounter, THREADED_COUNTER, NULL, thread);
-    if (rv != APR_SUCCESS)
-#endif
-    {
-        cur_unique_id.counter = counter;
-    }
+    new_unique_id.counter = cur_unique_id.counter;
     new_unique_id.stamp = htonl((unsigned int)apr_time_sec(r->request_time));
     new_unique_id.thread_index = htonl((unsigned int)r->connection->id);
 
@@ -250,6 +233,11 @@ static const char *gen_unique_id(const r
     }
     str[k++] = '\0';
 
+    /* and increment the identifier for the next call */
+
+    counter = ntohs(new_unique_id.counter) + 1;
+    cur_unique_id.counter = htons(counter);
+
     return str;
 }