You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2021/02/26 15:56:11 UTC

[Bug 65159] New: mod_unique_id generates non-unique ids

https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

            Bug ID: 65159
           Summary: mod_unique_id generates non-unique ids
           Product: Apache httpd-2
           Version: 2.4.46
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: mod_unique_id
          Assignee: bugs@httpd.apache.org
          Reporter: jonas.muentener@ergon.ch
  Target Milestone: ---

Created attachment 37746
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37746&action=edit
per-thread counter for mod_unique_id.c

mod_unique_id sometimes generates the same id for different requests.
We observed this behaviour about once every 5'000'000 requests (rough estimate)
under an average load of ~10'000 requests/s. All requests are HTTP/1.1
requests.

The supplied patch addresses this problem by using a per-thread counter.
Using this patch, we were no longer able to reproduce the issue.
Note that the patch is not compatible with Windows and thus only serves as a
proof of concept.
Alternatively, using atomic operations on the counter would likely solve the
problem too.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #31 from Ruediger Pluem <rp...@apache.org> ---
(In reply to Joe Orton from comment #28)
> (In reply to Michael Kaufmann from comment #27)
> > If you want to replace the implementation of mod_unique_id, a simple
> > approach would be to generate the whole ID with pseudo-random bytes, like
> > other web servers do. Unfortunately there is no fast APR function available
> > to generate pseudo-random bytes (e.g. reading from /dev/urandom on Linux),
> > just like there is no APR function available for real thread local storage.
> 
> Yes, I wondered about this too. But there is apr_generate_random_bytes(),
> also httpd has an RNG exposed via ap_random_*, plus we even have access to a
> UUID generator via apr_uuid_ which (should) be plugged through to
> getrandom() etc.  Having mod_ssl generate UNIQUE_ID via RAND_bytes() would
> also be a simpler alternative.

Is it a wise idea to make this id generation dependent on mod_ssl?
apr_generate_random_bytes / apr_uuid_ seem to offer a more independent
approach.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #44 from Atle Solbakken <at...@goliathdns.no> ---
(In reply to Atle Solbakken from comment #43)

> A database backend complains whenever there is a duplicate key. [...] There are two hosts (virtual) running the API, both of them get duplicate keys

Maybe it should be mentioned that the two hosts have their own database, the
collisions are not due to the same key being generated on two different hosts.

Atle.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #17 from WJCarpenter <bi...@carpenter.org> ---
FWIW, my local testing was only a couple dozen requests, and even our
heavily-loaded production server where this occurred is very unlikely to have
seen >65k requests in a single second, even during bursts of traffic. I think
the machine would probably catch on fire if that happened. :-)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #20 from Eric Covener <co...@gmail.com> ---
>  I still don't know the relationship between r->connection->id and the 
> current apr_thread_t.

Under the WINNT MPM, the worker threads are created in a simple loop and passed
their own index as an argument:

 997         for (i = 0; i < ap_threads_per_child; i++) {
 998             int *score_idx;
 999             int status = ap_scoreboard_image->servers[0][i].status;
1000             if (status != SERVER_GRACEFUL && status != SERVER_DEAD) {
1001                 continue;
1002             }
1003             ap_update_child_status_from_indexes(0, i, SERVER_STARTING,
NULL);
1004
1005             child_handles[i] = CreateThread(NULL, ap_thread_stacksize,
1006                                             worker_main, (void *) i,
1007                                             stack_res_flag, &tid);

This index becomes c->id. The c->id integer/offset is passed down as
"thread_num" when the connection is created:

Related: The c->current_thd is an apr_thread_t which is a wrapper around an OS
thread.


 819         c = ap_run_create_connection(context->ptrans, ap_server_conf,
 820                                      context->sock, thread_num, sbh,

Which ends up in:
5195 static conn_rec *core_create_conn(apr_pool_t *ptrans, server_rec *server,
5196                                   apr_socket_t *csd, long id, void *sbh,
5197                                   apr_bucket_alloc_t *alloc)
...
5250     c->id = id;

Note: the tid logged by mod_log_config is related to the OS thread wrapped in
the apr thread, not the simple index / c->id.

c->id would be used in the layout of the scoreboard and the order of the
mod_status output.


> BTW, the movement of threads in the table is not stress-related. I also saw it happen in during mostly idle times when the server was only seeing some lightweight status check requests once every few seconds.


AFAICT you should never see a different pair of c->id and c->current_thread on
Windows unless you restart.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #38 from WJCarpenter <bi...@carpenter.org> ---
Patch https://bz.apache.org/bugzilla/attachment.cgi?id=38021&action=diff looks
pretty good to me, too.

There's a lot of complexity still floating around in this module's code, but
I'd be happy with "if it ain't broke...." and use this simple solution.

I don't know enough to comment on whether or not to use apr_atomic_inc32().
Although an atomic increment is obviously good in theory, are there any
possible performance side effects?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #18 from WJCarpenter <bi...@carpenter.org> ---
(In reply to Michael Kaufmann from comment #16)
> Note that 16777216 is htonl(1) and 83886080 is htonl(5) on x86, so these are
> the threads with index 1 and index 5.

This was an interesting insight that got me to thinking. I added some things to
what I have been printing for each call.

    //    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01560)
    fprintf(stderr,
                 "Unique ID: ts: %u, ctr %u -> %u, tdx %u -> %u -> 0x%x, %s\n",
            new_unique_id.stamp,
            new_unique_id.counter, ntohs(new_unique_id.counter),
            new_unique_id.thread_index, ntohl(new_unique_id.thread_index),
thread,
            str);

In addition to decoding back to host format of the counter value and the thread
index, I also print the address of the apr_thread_t object for the current
thread. I again cranked down my thread pool to just 6 threads and re-ran my
curl-based test. Here are a couple examples of duplicates that cropped up:

Unique ID: ts: 1472666209, ctr 60362 -> 51947, tdx 0 -> 0 -> 0x459b7618,
YRrHVxmnrp_KPQCePqHK6wAAAAA
Unique ID: ts: 1472666209, ctr 60362 -> 51947, tdx 0 -> 0 -> 0x45eec678,
YRrHVxmnrp_KPQCePqHK6wAAAAA

Unique ID: ts: 499587681, ctr 60362 -> 51947, tdx 33554432 -> 2 -> 0x459bb638,
YRrHHRmnrp_KPQCePqHK6wAAAAI
Unique ID: ts: 499587681, ctr 60362 -> 51947, tdx 33554432 -> 2 -> 0x45f06748,
YRrHHRmnrp_KPQCePqHK6wAAAAI

For those pairs of requests, the thread index is the same, but the address of
the apr_thread_t object is different. This was seen among many normal-looking
entries that were enough to convince me that the apr_thread_t address that I
was printing was legit. Since it looks like r->connection->id is an index into
some table, something in Apache code is moving entries around in that table. It
is, therefore, not a very good choice as a proxy for the thread identity.

BTW, the movement of threads in the table is not stress-related. I also saw it
happen in during mostly idle times when the server was only seeing some
lightweight status check requests once every few seconds.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #6 from WJCarpenter <bi...@carpenter.org> ---
I don't know why the original poster saw the rare duplicates. Maybe it was
unlucky timing of threads overlapping in the (critical) section in the space
between grabbing the counter value and updating it.

Anyhow, I think for us the fix makes the problem much worse. When we upgraded
from 2.4.46 to 2.4.48, we started seeing duplicates quite often. It was noticed
by a team that harvests our log records and puts them into an external DB for
analytical purposes. In one case, where a security scanner kicked in, we saw
172 requests with the same unique id.

I have a theory about why it's worse now, but I'm not quite sure of my Apache
facts. Both before and after the patch, the thing that distinguishes one thread
from another is r->connection->id. I wasn't able to track down what that "id"
value actually comes from, except for a comment claiming it should be unique
for all time or something like that. But suppose that "id" does not change from
request to request. For example, for two consecutive requests over the same
persistent TCP connection, would they have the same r->connection->id value?
(That's the thing I don't know.)

Before the patch, there was a single global counter and that little window of
time which probably leaked duplicate unique IDs on rare occasions. After the
patch, we have a counter per worker thread, and that should eliminate the
problem of that little time window. But the patch initializes all of the
per-thread counters to the same or similar values (the otherwise unused global
counter). And that means that if r->connection->id is the same, or if it can
sometimes be the same for requests handled by different threads, then there
will be duplicate unique IDs within the same clock second.

Anybody able to comment on my hypothesis about why this happens for us?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #24 from Atle Solbakken <at...@goliathdns.no> ---

Hi in #65307 I've suggested a patch in attempt to help with the issues with the
counter. This is the same patch as in the GitHub PR mentioned by Joe.

The following fields are used to produce an ID.

- Process ID of the running process
- Thread ID of the running thread
- Timestamp in microseconds
- 16 bit incrementing counter for each thread
- 16 bit server ID manually set in UniqueIdServerId (defaults to 0)

The process ID is derived using getpid().

The thread ID is derived using some a technique borrowed from server/log.c,
basically apr_os_thread_current() is used. I'm not 100% sure if this will
always produce a unique number within each process but somebody else might know
this.

The timestamp is derived using apr_time_now().

The early versions of mod_unique_id ensured global uniqueness using
IP-addresses of the server and client. This has since has been removed as this
no longer works and a random number is now used. I suggest to set a server ID
manually in the configuration file to guarantee uniqueness across servers,
these IDs must be set up to actually be unique among servers which share the
same backend/DB etc. (in environments where collisions are not acceptable).

The counter in this case is unique for each thread which avoids problems with
concurrency, this is already in place after the last change.

If threads are not enabled, a static counter is used which will then be per
process.

The code is set up to use the new function apr_encode_base64 in newer versions.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #13 from WJCarpenter <bi...@carpenter.org> ---
With the "#if 0" version of the code, I tried some experiments with curl making
parallel requests (you need a recent version of curl to get the "-Z" option).
Over a few trials, I did see a couple of entries where the counter value was
the same. But in each case, the thread index was different, so the generated
unique ID was OK.

It's a complete side-issue, but anybody know why the code uses htons(counter)?
Maybe there is a reason for it, but I don't see why we need the opaque value to
be in network byte order. (If faked me out temporarily because I was seeing the
counter go up by a couple hundred for each call.)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|RESOLVED                    |REOPENED

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

Michael Kaufmann <ap...@michael-kaufmann.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|REOPENED                    |RESOLVED

--- Comment #41 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
This bug has been fixed in Apache httpd 2.4.49.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #2 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
The new patch looks good to me. Nice work, thank you very much!

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #42 from WJCarpenter <bi...@carpenter.org> ---
I've finally had a chance to re-try the quickie curl-based test that showed the
original regression. No duplicates in my test. Thanks to all who contributed to
the 2.4.49 fix.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #26 from Atle Solbakken <at...@goliathdns.no> ---
(In reply to WJCarpenter from comment #25)
> Setting a unique identity for the server might be a little inconvenient.
> Many high-traffic sites deploy multiple httpd instances with exactly the
> same httpd.conf file. 

That is true, there would in that case have to be some form of automatic
setting of the identity during deployment. Maybe also 16 bits for this value
would be too short.

In any case I've tried to make it easy to change the actual composition of the
ID in the code.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

Yann Ylavic <yl...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #38021|0                           |1
        is obsolete|                            |

--- Comment #39 from Yann Ylavic <yl...@gmail.com> ---
Created attachment 38022
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38022&action=edit
Atomic counter

This is the patch finally proposed for backport.

I don't really anticipate an overhead for the atomic operation, it should be in
the noise (MPM event makes use of atomics for instance already to schedule the
connections).

It's in any case lighter than a system call to getrandom() or "/dev/urandom"
for instance, though we might need to use that (directly or via apr_uuid_get())
or a (sip)hash at some point to add more randomness to the ID (as Joe or Eric
said).

This leaves the question of how much we can/want to grow the unique ID size,
it's currently 27 characters (uuencoding of 20 bytes), 32 characters would be
better IMHO as the uuencoding of following 24 bytes:
- uint32_t timestamp in seconds (for compatibility)
- char root[12] for the long term unicity of the child process
- uint64_t siphash of a 32bit counter and 64bit/microseconds timestamp and...
  (the 128bit secret for siphash would be randomly generated at child_init)

Or we could simply use:
- uint32_t timestamp in seconds (for compatibility)
- char data[16] from apr_uuid_get() or apr_generate_random_bytes()
which is still 20 bytes, though possibly a bit longer to generate depending on
the platform..

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #16 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
Note that 16777216 is htonl(1) and 83886080 is htonl(5) on x86, so these are
the threads with index 1 and index 5.

The htonl() is there to ensure that little-endian machines and big-endian
machines generate the ID in the same way. Probably it does not matter too much
in practice.

The counter is only 16 bits, so maybe one thread handles more than 65535
requests per second? (sounds unrealistic)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #10 from Christophe JAILLET <ch...@wanadoo.fr> ---
Thx,

Do you see other values for ctr and/or txd?

tdx 16777216d is odd.
16777216 = 1 00000000 00000000 00000000
So I guess, that in your case, the same thread process all the requests. Why
not.

ctr 41467d
This looks to be always the same value. This shouldn't.



What also puzzles me is:
   - in the doc of "mod_uunique_id": "First a brief recap of how the Apache
server works on Unix machines. This feature currently isn't supported on
Windows NT."
     I don't see why it wouldn't be supported on Windows.
     From my point of view, everything is in place for it to work


Apparently you are able to compile your own windows version.
Could you just confirm that changing the 2 "#if APR_HAS_THREADS" by "#if 0" in
mod_unique_id.c works for you?
It should revert to the previous behaviour. (In fact, the time window for
duplicate should even be reduced. So if you were seeing some duplicate from
time to time before, you should see less)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

Atle Solbakken <at...@goliathdns.no> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|RESOLVED                    |REOPENED

--- Comment #43 from Atle Solbakken <at...@goliathdns.no> ---
Hi all

I'm still getting collisions in a production environment. This happens with an
API where requests come from clients in small bursts, the clients access
multiple endpoints regularly. For every request, a session is created using the
unique ID as key. A database backend complains whenever there is a duplicate
key.

There are two hosts (virtual) running the API, both of them get duplicate keys.
They are running 2.4.53-1~deb11u1 which should have the latest patches from
last year. There is no special configuration of Apache on these hosts, but they
use MPM prefork as this is the default when using PHP.

I don't actively try to produce duplicates, it has happened with as little as
three requests per second. We get duplicates in about 1 in 45.000 requests.

A small number of collisions doesn't actually matter in our situation, but
'mod_unqiue_id' does still produce non-unique IDs :)

Atle.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #32 from WJCarpenter <bi...@carpenter.org> ---
Based on tremendous success with an unrelated project, I really like the idea
of just using random bytes, and it's OK with me if it's random bytes for the
whole thing.

But another lesson we learned in that unrelated project is that you need a good
quality PRNG. (For example, compare Java Random to SecureRandom.) That said, I
don't know anything about the quality of AP/APR methods for random bytes.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #28 from Joe Orton <jo...@redhat.com> ---
(In reply to Michael Kaufmann from comment #27)
> If you want to replace the implementation of mod_unique_id, a simple
> approach would be to generate the whole ID with pseudo-random bytes, like
> other web servers do. Unfortunately there is no fast APR function available
> to generate pseudo-random bytes (e.g. reading from /dev/urandom on Linux),
> just like there is no APR function available for real thread local storage.

Yes, I wondered about this too. But there is apr_generate_random_bytes(), also
httpd has an RNG exposed via ap_random_*, plus we even have access to a UUID
generator via apr_uuid_ which (should) be plugged through to getrandom() etc. 
Having mod_ssl generate UNIQUE_ID via RAND_bytes() would also be a simpler
alternative.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #37 from Christophe JAILLET <ch...@wanadoo.fr> ---
(In reply to Yann Ylavic from comment #36)
> How about this patch?
FWIW, +1

It has also been discussed that 'apr_atomic_inc32()' could also be cheap and
could definitively close the hole.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #12 from WJCarpenter <bi...@carpenter.org> ---
I recompiled with "#if 0", as suggested. It seems to eliminate the current
repro that I have. That's expected and takes us back to something close to the
race condition that existed before the per-thread counters.

Is it too late to go back to the 2.4.46 code and try to close the race
condition? I don't know if the Apache/APR ecosystem has atomic operations. Even
if not, doing "++" on a static short ought to be a pretty narrow window.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #19 from WJCarpenter <bi...@carpenter.org> ---
I'm not sure of the best way to move this along. Should I just wait for a
committer to come up with a patch, or would it help with resolution if I came
up with a patch and posted it here? Or would that just be a distraction? 

(For our production environments, we're going to revert to the 2.4.46 version
of mod_unique_id for now since that is less problematic for our usage.)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

Yann Ylavic <yl...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Yann Ylavic <yl...@gmail.com> ---
Backported to 2.4.47 in r1887386.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #35 from WJCarpenter <bi...@carpenter.org> ---
That does narrow the time window. I wonder why it's not narrowed even further
by just changing this line:

  new_unique_id.counter = cur_unique_id.counter;

to

  new_unique_id.counter = cur_unique_id.counter++;

Instead, it does through a htons() conversion followed by a ntohs() conversion
before achieving the same thing.

Perhaps I am overlooking some multi-threading consideration that makes the
simple post-increment unwise?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #11 from WJCarpenter <bi...@carpenter.org> ---
First a comment about my repro scenario. I used HTTP/1.0 because I wanted curl
to avoid reusing the connection. Of course, we don't really see production
HTTP/1.0 traffic these days (or at least not in any quantity to care about).
Today I repeated the experiment but invoked curl (defaulting to HTTP/1.1) many
times in a loop instead of listing the URL many times on the command line. That
gave similar repro.

Unique ID: ts: 3844085857d, ctr 18097d, tdx 83886080d,
YRgg5a4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3844085857d, ctr 18097d, tdx 83886080d,
YRgg5a4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3844085857d, ctr 18097d, tdx 83886080d,
YRgg5a4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3844085857d, ctr 18097d, tdx 83886080d,
YRgg5a4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3844085857d, ctr 18097d, tdx 83886080d,
YRgg5a4m1ckw_QogJNqxRgAAAAU

Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3860863073d, ctr 18097d, tdx 83886080d,
YRgg5q4m1ckw_QogJNqxRgAAAAU

Unique ID: ts: 3877640289d, ctr 18097d, tdx 83886080d,
YRgg564m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3877640289d, ctr 18097d, tdx 83886080d,
YRgg564m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3877640289d, ctr 18097d, tdx 83886080d,
YRgg564m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3877640289d, ctr 18097d, tdx 83886080d,
YRgg564m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3877640289d, ctr 18097d, tdx 83886080d,
YRgg564m1ckw_QogJNqxRgAAAAU
Unique ID: ts: 3877640289d, ctr 18097d, tdx 83886080d,
YRgg564m1ckw_QogJNqxRgAAAAU

As Christophe JAILLET noted last time, the counter value stays the same. It's
only the tick of the clock that makes a difference. If the thread index
(r->connection->id) really does reflect the worker thread, then it looks like
those requests were all handled by the same thread. I can see that the counter
does change for particular thread indexes outside of my testing, so it's not a
case of it being completely stuck.

That's interesting about it not being supported on Windows NT. I hadn't noticed
that before. Somebody before my time started using this module years ago,
possibly when Windows was the only supported platform for us. I agree, there
doesn't seem to be any reason for that restriction with the current code, but
the rest of that documentation page describes some things that are no longer
true for the current code. For example, host names / IP addresses aren't part
of the input any more.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

WJCarpenter <bi...@carpenter.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bill-apache@carpenter.org

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #22 from WJCarpenter <bi...@carpenter.org> ---
Has anyone else got the problem to repro (or tried and failed) on Windows or
Linux?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #25 from WJCarpenter <bi...@carpenter.org> ---
Setting a unique identity for the server might be a little inconvenient. Many
high-traffic sites deploy multiple httpd instances with exactly the same
httpd.conf file. And some of those sites probably scale up and down dynamically
the number of instances based on expected load based on day and time. It's an
otherwise attractive idea.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #14 from Christophe JAILLET <ch...@wanadoo.fr> ---
(In reply to WJCarpenter from comment #12)
> I recompiled with "#if 0", as suggested. It seems to eliminate the current
> repro that I have. That's expected and takes us back to something close to
> the race condition that existed before the per-thread counters.

Thx. It was only to confirm that we are looking at the right direction.
Looks like a Windows specific issue.

> Is it too late to go back to the 2.4.46 code and try to close the race
> condition?

The goal of the change was to eliminate the race. If there is a better solution
than what is in 2.4.48, close to 2.4.46 code or not, there is no problem to
implement it in a future release.

> I don't know if the Apache/APR ecosystem has atomic operations.

Yes, atomic operations are available. See
http://apr.apache.org/docs/apr/1.6/group__apr__atomic.html

When I wrote the patch, I was unsure that apr_atomic_add32() was "really"
atomic. (i.e the read AND the add AND the write).
Looking at a few implementation, this seems to be the case, so I guess that
this could be used instead, to avoid the thread based mechanism.

> Even if not, doing "++" on a static short ought to be a pretty narrow window.

In 2.4.48, the !APR_HAS_THREADS path is close to it. We can narrow even more
the window by slightly reordering the code.
But when I wrote the 2.4.48 patch, my goal was to remove the race, not to
narrow the window :)


(In reply to WJCarpenter from comment #13)
> It's a complete side-issue, but anybody know why the code uses htons(counter)?
> Maybe there is a reason for it, but I don't see why we need the opaque value
> to be in network byte order

I had the exact same question. I left it because I had no real answer.
I also think that it is useless.




In the comments in the 2.4.46 code, there was also something about performance.
> /*
> * 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.
> */
This is an OLD comment. I don't have any numbers to see how much it can be true
but I don't think that it really matters nowadays.


I'm fan of the KISS principle ([1]), so if a solution with atomic works, I
think it would be a cleaner solution.


[1]: https://en.wikipedia.org/wiki/KISS_principle

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #8 from Christophe JAILLET <ch...@wanadoo.fr> ---
Hi,

What MPM are you using?
Could you provide a few examples of UNIQUE_ID that are generated?

Thx

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #33 from Christophe JAILLET <ch...@wanadoo.fr> ---
(In reply to Christophe JAILLET from comment #4)
> Applied on trunk in r1887244 and r1887245.
> 

This has been removed from trunk in r1892914 because of the Windows regression.
The time window where the duplicate can be generated has been proposed in
r1892915.

Both have been proposed to backport in 2.4.x

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #29 from Joe Orton <jo...@redhat.com> ---
Created attachment 38009
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38009&action=edit
use OpenSSL PRNG for unique_id generation

Proof of concept.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #30 from Eric Covener <co...@gmail.com> ---
How about a configurable way to mix-in a smaller amount of random data from an
optional function?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #3 from Jonas Müntener <jo...@ergon.ch> ---
I tested the patch for several hours and did not encounter any duplicate IDs,
so it looks good to me. Nice work!

Thank you for the prompt reply and for taking the time to write the patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #4 from Christophe JAILLET <ch...@wanadoo.fr> ---
Applied on trunk in r1887244 and r1887245.

Proposed for backport in 2.4.x in r1887246.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #1 from Christophe JAILLET <ch...@wanadoo.fr> ---
Created attachment 37749
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37749&action=edit
Alternative patch

Hi,

Your patch LGTM, with the limitation you mention.
I've not seen any "simple" workaround.


Attached is a variation which uses the APR functions in order to store some
data (i.e. the counter) on a thread basis.

It also:
 - removes some useless htons/ntohs conversion
 - reduce the time window when duplicate can be generated if we can not store
the counter value in each thread
 - axe some old comments

Tested with the test framework.
There could be a slight performance penalty because it adds an additional hash
table lookup.


Comments welcomed!

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #36 from Yann Ylavic <yl...@gmail.com> ---
Created attachment 38021
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38021&action=edit
Counter in network byte order for uuencoding only

How about this patch?

The counter remains in host byte order in cur_unique_id so that we can
cur_unique_id.counter++, and the htons() is done outside the "critical section"
so that the counter remains in network byte order for the uuencoding.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #21 from Eric Covener <co...@gmail.com> ---
in my windows sandbox I see a very unusual affinity for the highest index
threads to always grab new connections (unless I hang them with a slow
response, then the next highest indexed seems to get used).

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #27 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
Created attachment 37987
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37987&action=edit
Performance improvement for mod_unique_id

The root cause of the problem is that the winnt MPM creates multiple APR thread
objects for a single OS thread, since r1374874. Reverting that commit fixes the
issues with non-unique IDs on Windows. But just reverting the commit is
probably not good enough.

While debugging the problem, I have created a patch that improves the
performance: With the patch, apr_thread_data_set() gets called only when
necessary. So maybe this patch could be applied after the problem with the
winnt MPM has been fixed.

If you want to replace the implementation of mod_unique_id, a simple approach
would be to generate the whole ID with pseudo-random bytes, like other web
servers do. Unfortunately there is no fast APR function available to generate
pseudo-random bytes (e.g. reading from /dev/urandom on Linux), just like there
is no APR function available for real thread local storage.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #7 from WJCarpenter <bi...@carpenter.org> ---
FYI, for our scenario, all traffic goes through a reverse proxy load balancer
before it gets to Apache httpd. So, if r->connection->id depends on IP
addresses or ports or whatnot, then the pool is kind of small for that
toplology.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #9 from WJCarpenter <bi...@carpenter.org> ---
We use the default MPMs, so winnt for Windows and (I think) worker for Linux.
Apache httpd 2.4.48.

So far, we have only observed this on Windows, but we haven't been able to rule
out that it happens on Linux. We observed it on a production system, not a load
test. 

I have been working on trying to make a simple repro. The one I have so far
cranks down the number of threads on Windows to a very small number (6) and
then sends a few dozen requests with a long curl command line specifying
HTTP/1.0. No LB or reverse proxy in the path. You don't have to tell me that
that's not a realistic scenario in 2021. I'm just trying to prove it happens
sometimes in a reproducible way. The same test has not yet produced a repro on
Linux.

I added some logging of the interesting parts of the unique_id_rec (the
timestamp, the counter, and the "thread index" that comes from
r->connection->id). Here is an example of a duplicate that happened in my test
(ignore the "d" after each integer ... some clown programmer got the format
wrong in the printf):

Unique ID: ts: 2986088033d, ctr 41467d, tdx 16777216d,
YRb8sYYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 2986088033d, ctr 41467d, tdx 16777216d,
YRb8sYYFmO-FZbwnLY37oQAAAAE

Unique ID: ts: 3002865249d, ctr 41467d, tdx 16777216d,
YRb8soYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3002865249d, ctr 41467d, tdx 16777216d,
YRb8soYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3002865249d, ctr 41467d, tdx 16777216d,
YRb8soYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3002865249d, ctr 41467d, tdx 16777216d,
YRb8soYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3002865249d, ctr 41467d, tdx 16777216d,
YRb8soYFmO-FZbwnLY37oQAAAAE

Unique ID: ts: 3019642465d, ctr 41467d, tdx 16777216d,
YRb8s4YFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3019642465d, ctr 41467d, tdx 16777216d,
YRb8s4YFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3019642465d, ctr 41467d, tdx 16777216d,
YRb8s4YFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3019642465d, ctr 41467d, tdx 16777216d,
YRb8s4YFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3019642465d, ctr 41467d, tdx 16777216d,
YRb8s4YFmO-FZbwnLY37oQAAAAE

Unique ID: ts: 3036419681d, ctr 41467d, tdx 16777216d,
YRb8tIYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3036419681d, ctr 41467d, tdx 16777216d,
YRb8tIYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3036419681d, ctr 41467d, tdx 16777216d,
YRb8tIYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3036419681d, ctr 41467d, tdx 16777216d,
YRb8tIYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3036419681d, ctr 41467d, tdx 16777216d,
YRb8tIYFmO-FZbwnLY37oQAAAAE

Unique ID: ts: 3053196897d, ctr 41467d, tdx 16777216d,
YRb8tYYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3053196897d, ctr 41467d, tdx 16777216d,
YRb8tYYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3053196897d, ctr 41467d, tdx 16777216d,
YRb8tYYFmO-FZbwnLY37oQAAAAE
Unique ID: ts: 3053196897d, ctr 41467d, tdx 16777216d,
YRb8tYYFmO-FZbwnLY37oQAAAAE

Those were on Windows with these httpd.conf items:

MaxConnectionsPerChild 0
ThreadsPerChild 6
KeepAlive On
MaxKeepAliveRequests 100
KeepAliveTimeout 8

Here's how I printed the above lines (right before "return str" in
gen_unique_id())

    //    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01560)
    fprintf(stderr,
                 "Unique ID: ts: %ud, ctr %ud, tdx %ud, %s\n",
                  new_unique_id.stamp, new_unique_id.counter,
new_unique_id.thread_index, str);

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

Michael Kaufmann <ap...@michael-kaufmann.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |apache-bugzilla@michael-kau
                   |                            |fmann.ch

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #34 from Christophe JAILLET <ch...@wanadoo.fr> ---
(In reply to Christophe JAILLET from comment #33)
> The time window where the duplicate can be generated has been proposed in
> r1892915.

The time window where the duplicate can be generated has been *narrowed* in
r1892915.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #40 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
After the "Atomic counter" patch, the documentation in the comments of
mod_unique_id.c is outdated. The counter is now per process, not per thread. So
it can overflow if a process handles more than 65535 requests per second,
previously it overflowed if a single thread handled this number of requests.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #15 from WJCarpenter <bi...@carpenter.org> ---
> > I don't know if the Apache/APR ecosystem has atomic operations.
> 
> Yes, atomic operations are available. See
> http://apr.apache.org/docs/apr/1.6/group__apr__atomic.html

I don't know if using that is even necessary. Without looking at the generated
machine code, I would expect that pre-incrementing a short is either atomic on
modern architectures or is usually of an acceptably small window of time.

Another way to eliminate the race could be to make the "thread index" a true
indicator of the thread. I still don't know the relationship between
r->connection->id and the current apr_thread_t. So, I don't know the difficulty
of switching to that approach.

> I'm fan of the KISS principle ([1]), so if a solution with atomic works, I
> think it would be a cleaner solution.

Me, too. I don't want to suggest a big change for the duplicates we started
noticing if we can find a nice simple change to the 2.4.46 code that resolves
the problem that triggered the patch. 

The code even in 2.4.46 has some complications that make it tricky to follow.
It builds up a unique ID record and then goes to a lot of trouble to encode
just the bytes that matter and not any padding bytes. Maybe the original author
anticipated that unique ID record becoming more complex over time. As it is, it
would be simpler to keep track of the per-process ROOT in a static, keep the
counter as another static, and then just directly encode that small number of
pieces. Or copy that small number of pieces into a scratch buffer if that makes
the encoding loop simpler. The entire unique ID record is subject to the race
condition, but it's only the counter that makes any difference in that race.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 65159] mod_unique_id generates non-unique ids

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=65159

--- Comment #23 from Joe Orton <jo...@redhat.com> ---
FYI there is a rewrite of the whole module contributed here: 

https://github.com/apache/httpd/pull/185

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org