https://bz.apache.org/bugzilla/show_bug.cgi?id=65159
--- Comment #14 from Christophe JAILLET <[email protected]> --- (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: [email protected] For additional commands, e-mail: [email protected]
