Hi Micha, thanks for the review.
On Wed, Mar 25, 2015 at 7:19 AM, Micha Lenk <mi...@lenk.info> wrote: > > I only had time to review the code changes. I looked at the > implementation of apr_global_mutex_timedlock(). There I noticed that, if > APR_HAS_THREADS, you are doing 3 timed operations without checking the > remaining time. If all operations take almost the given timeout, this > can in the end result in almost 3 times the allowed value. The new _timedlocked() functions all take a third parameter (called absolute) which tells whether the given timeout is absolute (1) or relative (0). This is because some native implementations use an absolute time, and others a relative one, hence we do the conversion in the implementations when needed only. In apr_global_mutex_timedlock(), if the given timeout is relative, we force it to an absolute one and call apr_thread_mutex_timedlock() (if needed) and apr_proc_mutex_timedlock() accordingly, hence the timeout is assured globally. > > To mitigate that design flaw I would provide the timeout by reference > and update it by the functions using it. This has also the nice benefit > that the caller is able to retrieve the time it needed to wait. By doing so, you have to get the current time (call apr_time_now()) each time, and I wanted to avoid it when the native functions don't need it. The remaining time is not important if you can pass an absolute time (IMHO). Regards, Yann.
