Re: How to wait on a global lock with timeout

2015-03-25 Thread Micha Lenk
Hi Yann,

Am 20.03.2015 um 02:59 schrieb Yann Ylavic:
 a few times later, commited in [1] and [2] :)

Wow, thanks for coming back to that issue.

 Can you please check if it works for you?

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.

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.

I hope to have some time to check out the new functions in my code soon.

 I also attached the patch against 1.6.x, if that can help...
 It should also apply to 1.5.x, though it will never be backported
 there (due to API changes).

Backports are not needed. Thanks for offering.

Regards,
Micha


Re: How to wait on a global lock with timeout

2015-03-25 Thread Yann Ylavic
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.