You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2020/03/06 16:15:18 UTC
svn commit: r1874909 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS
modules/http2/h2_bucket_beam.c modules/http2/h2_bucket_beam.h
modules/http2/h2_task.c modules/http2/h2_util.c modules/http2/h2_version.h
Author: jim
Date: Fri Mar 6 16:15:17 2020
New Revision: 1874909
URL: http://svn.apache.org/viewvc?rev=1874909&view=rev
Log:
Merge r1874689 from trunk:
*) mod_http2: Fixes issue where mod_unique_id would generate non-unique request
identifier under load, see <https://github.com/icing/mod_h2/issues/195>.
[Michael Kaufmann, Stefan Eissing]
Submitted by: icing
Reviewed by: icing, ylavic, jim
Modified:
httpd/httpd/branches/2.4.x/ (props changed)
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/STATUS
httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.c
httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.h
httpd/httpd/branches/2.4.x/modules/http2/h2_task.c
httpd/httpd/branches/2.4.x/modules/http2/h2_util.c
httpd/httpd/branches/2.4.x/modules/http2/h2_version.h
Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
Merged /httpd/httpd/trunk:r1874689
Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1874909&r1=1874908&r2=1874909&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Fri Mar 6 16:15:17 2020
@@ -1,6 +1,10 @@
-*- coding: utf-8 -*-
Changes with Apache 2.4.42
+ *) mod_http2: Fixes issue where mod_unique_id would generate non-unique request
+ identifier under load, see <https://github.com/icing/mod_h2/issues/195>.
+ [Michael Kaufmann, Stefan Eissing]
+
*) mod_proxy_hcheck: Allow healthcheck expressions to use %{Content-Type}.
PR64140. [Renier Velazco <renier.velazco upr.edu>]
Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1874909&r1=1874908&r2=1874909&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Fri Mar 6 16:15:17 2020
@@ -146,12 +146,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
2.4.x patch: svn merge -c 1874323 ^/httpd/httpd/trunk .
+1: ylavic, gsmith, rpluem
- *) mod_http2: Fixes issue where mod_unique_id would generate non-unique request
- identifier under load, see <https://github.com/icing/mod_h2/issues/195>.
- trunk patch: http://svn.apache.org/r1874689
- 2.4.x patch: svn merge -c 1874689 ^/httpd/httpd/trunk .
- +1: icing, ylavic, jim
-
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
[ New proposals should be added at the end of the list ]
Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.c?rev=1874909&r1=1874908&r2=1874909&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.c Fri Mar 6 16:15:17 2020
@@ -196,7 +196,7 @@ static apr_bucket *h2_beam_bucket(h2_buc
* bucket beam that can transport buckets across threads
******************************************************************************/
-static void mutex_leave(void *ctx, apr_thread_mutex_t *lock)
+static void mutex_leave(apr_thread_mutex_t *lock)
{
apr_thread_mutex_unlock(lock);
}
@@ -217,7 +217,7 @@ static apr_status_t enter_yellow(h2_buck
static void leave_yellow(h2_bucket_beam *beam, h2_beam_lock *pbl)
{
if (pbl->leave) {
- pbl->leave(pbl->leave_ctx, pbl->mutex);
+ pbl->leave(pbl->mutex);
}
}
Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.h?rev=1874909&r1=1874908&r2=1874909&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.h (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_bucket_beam.h Fri Mar 6 16:15:17 2020
@@ -126,12 +126,11 @@ typedef struct {
* buffers until the transmission is complete. Star gates use a similar trick.
*/
-typedef void h2_beam_mutex_leave(void *ctx, struct apr_thread_mutex_t *lock);
+typedef void h2_beam_mutex_leave(struct apr_thread_mutex_t *lock);
typedef struct {
apr_thread_mutex_t *mutex;
h2_beam_mutex_leave *leave;
- void *leave_ctx;
} h2_beam_lock;
typedef struct h2_bucket_beam h2_bucket_beam;
Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_task.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_task.c?rev=1874909&r1=1874908&r2=1874909&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_task.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_task.c Fri Mar 6 16:15:17 2020
@@ -555,37 +555,36 @@ apr_status_t h2_task_do(h2_task *task, a
task->worker_started = 1;
if (c->master) {
- /* Each conn_rec->id is supposed to be unique at a point in time. Since
+ /* See the discussion at <https://github.com/icing/mod_h2/issues/195>
+ *
+ * Each conn_rec->id is supposed to be unique at a point in time. Since
* some modules (and maybe external code) uses this id as an identifier
* for the request_rec they handle, it needs to be unique for slave
* connections also.
- * The connection id is generated by the MPM and most MPMs use the formula
- * id := (child_num * max_threads) + thread_num
- * which means that there is a maximum id of about
- * idmax := max_child_count * max_threads
- * If we assume 2024 child processes with 2048 threads max, we get
- * idmax ~= 2024 * 2048 = 2 ** 22
- * On 32 bit systems, we have not much space left, but on 64 bit systems
- * (and higher?) we can use the upper 32 bits without fear of collision.
- * 32 bits is just what we need, since a connection can only handle so
- * many streams.
+ *
+ * The MPM module assigns the connection ids and mod_unique_id is using
+ * that one to generate identifier for requests. While the implementation
+ * works for HTTP/1.x, the parallel execution of several requests per
+ * connection will generate duplicate identifiers on load.
+ *
+ * The original implementation for slave connection identifiers used
+ * to shift the master connection id up and assign the stream id to the
+ * lower bits. This was cramped on 32 bit systems, but on 64bit there was
+ * enough space.
+ *
+ * As issue 195 showed, mod_unique_id only uses the lower 32 bit of the
+ * connection id, even on 64bit systems. Therefore collisions in request ids.
+ *
+ * The way master connection ids are generated, there is some space "at the
+ * top" of the lower 32 bits on allmost all systems. If you have a setup
+ * with 64k threads per child and 255 child processes, you live on the edge.
+ *
+ * The new implementation shifts 8 bits and XORs in the worker
+ * id. This will experience collisions with > 256 h2 workers and heavy
+ * load still. There seems to be no way to solve this in all possible
+ * configurations by mod_h2 alone.
*/
- int slave_id, free_bits;
-
- task->id = apr_psprintf(task->pool, "%ld-%d", c->master->id,
- task->stream_id);
- if (sizeof(unsigned long) >= 8) {
- free_bits = 32;
- slave_id = task->stream_id;
- }
- else {
- /* Assume we have a more limited number of threads/processes
- * and h2 workers on a 32-bit system. Use the worker instead
- * of the stream id. */
- free_bits = 8;
- slave_id = worker_id;
- }
- task->c->id = (c->master->id << free_bits)^slave_id;
+ task->c->id = (c->master->id << 8)^worker_id;
}
h2_beam_create(&task->output.beam, c->pool, task->stream_id, "output",
Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_util.c?rev=1874909&r1=1874908&r2=1874909&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_util.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_util.c Fri Mar 6 16:15:17 2020
@@ -1923,7 +1923,8 @@ int h2_util_frame_print(const nghttp2_fr
case NGHTTP2_GOAWAY: {
size_t len = (frame->goaway.opaque_data_len < s_len)?
frame->goaway.opaque_data_len : s_len-1;
- memcpy(scratch, frame->goaway.opaque_data, len);
+ if (len)
+ memcpy(scratch, frame->goaway.opaque_data, len);
scratch[len] = '\0';
return apr_snprintf(buffer, maxlen, "GOAWAY[error=%d, reason='%s', "
"last_stream=%d]", frame->goaway.error_code,
Modified: httpd/httpd/branches/2.4.x/modules/http2/h2_version.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http2/h2_version.h?rev=1874909&r1=1874908&r2=1874909&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http2/h2_version.h (original)
+++ httpd/httpd/branches/2.4.x/modules/http2/h2_version.h Fri Mar 6 16:15:17 2020
@@ -27,7 +27,7 @@
* @macro
* Version number of the http2 module as c string
*/
-#define MOD_HTTP2_VERSION "1.15.5"
+#define MOD_HTTP2_VERSION "1.15.8"
/**
* @macro
@@ -35,6 +35,6 @@
* release. This is a 24 bit number with 8 bits for major number, 8 bits
* for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
*/
-#define MOD_HTTP2_VERSION_NUM 0x010f05
+#define MOD_HTTP2_VERSION_NUM 0x010f08
#endif /* mod_h2_h2_version_h */