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",
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,
projects / thirdparty / apache / httpd.git / commitdiff
