On Friday 04 January 2013, Daniel Lescohier wrote: > I entered a bug on the thread-safety of the time conversion caches > in server/util_time.c and modules/loggers/mod_log_config.c. > > In brief, they're not thread-safe because: > > 1. The algorithm depends on total memory ordering, and both the > volatile qualifier and memory barriers are not used. > 2. The algorithm is subject to the "ABA problem."
I agree that the lack of memory barriers is a problem. And it ABA problem would not exist if callers of ap_recent_* would actually check that the time *is* recent (as written in the docs in util_time.h). This is not a problem for the error log, because it always uses apr_time_now(). But the request time in mod_log_config may be further in the past. Out of interest, have you seen the function give wrong results in practice? > The details of the problem are here: > > https://issues.apache.org/bugzilla/show_bug.cgi?id=54363 > > I included in the bug not-yet-tested patches with a different > algorithm. > > Do other people agree that the existing algorithm have the problems > explained in detail in the bug? I haven't reviewed your patches in detail, yet. One thing: apr_atomic_read should be enough, you don't need apr_atomic_cas with the same value. Cheers, Stefan
