apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: lockmech ambiguity (Was: Release _timedlock API in 1.6.x?)
Date Thu, 01 Jun 2017 19:59:49 GMT
Hi Bill,

On Thu, Jun 1, 2017 at 6:32 PM, William A Rowe Jr <wrowe@rowe-clan.net> wrote:
> On Mon, May 22, 2017 at 3:22 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>> From an API perspective, the single timeout argument looks consensual.
>> So there may still be some bug(s) somewhere, which we would fix as
>> always, but it would unlikely break existing applications since it's
>> new feature, hence my positive vote...
> I'm quite confused in retrospect with the apr_lockmech_e logic
> as it exists in the 'oddball' architectures and the enum values.
> APR_LOCK_DEFAULT looks to be the correct answer on the
> oddball architectures. When we do an apr_os_proc_mutex_get_ex
> we want to know what sort of void* object was just returned to us.
> So long as the underlying lock object type didn't change (which it
> doesn't appear to do so on the single-mutex-style architectures),
> the right answer seems to remain APR_LOCK_DEFAULT.
> APR_LOCK_DEFAULT_TIMED seems to express the desire to
> have a lock mech that can be timed. But the underlying system
> object doesn't vary, right? So apr should still report that it is one
> APR_LOCK_DEFAULT. This could have been broken down into
> explicit WIN32, NETWARE, BEOSSEM types. That seems like
> an apr 2.0 change if we want it to happen.

APR_LOCK_DEFAULT_TIMED is a whish for apr_proc_mutex_create() or/and
apr_os_proc_mutex_set_ex(), but also (and soncequently) a capability
of the underlying OS mutex.

For non-unixes mutexes, appart from Netware, the underlying OS mutex
can be apr_proc_mutex_timedlock()ed natively so why wouldn't we
announce it as APR_LOCK_DEFAULT_TIMED?

For Netware, we could create the associated condvar at
apr_os_proc_mutex_set_ex() time if we also wanted to make it
APR_LOCK_DEFAULT_TIMED, but I didn't spend too much time on Netware
(and couldn't test anything...).
Anyway, Netware proc mutexes are APR_LOCK_DEFAULT still.

> As a short-term hack on these architectures, defining the enum
> value of APR_LOCK_DEFAULT_TIMED as identical to the
> APR_LOCK_DEFAULT value, both IsKindOf tests and the OS
> datatype handling would remain correct, right?

So yes, I think that for OSes which have a single mech which is TIMED
natively we should APR_LOCK_DEFAULT_TIMED = APR_LOCK_DEFAULT, makes
sense and avoids breaking existing APR_LOCK_DEFAULT usages.

> On Unix, the problem in 1.7.x is broader... since a number of the
> lockmech can be timed locks (and a number cannot). Again,
> having retrieved the apr_os_proc_mutex, we need to know which
> API handles it, not whether it is APR_LOCK_DEFAULT[_TIMED].

This is the case actually, apr_os_proc_mutex_get_ex() returns
mutex->method->mech, which is the real mech.

> We might -also- want to know whether it is the default, and
> whether it supported timed attempts (but if you know the API,
> you know if there are timeouts available.)

There could be a (some) helper(s) for that.

> Seems we heavily overloaded the recent timedlock and lockmech
> changes to try to accomplish multiple purposes which are at odds
> with one another, and need to mop this up before we can declare
> 1.7.x baked.

I don't find it odd, still, and personally I prefer a ENOTIMPL when
APR_LOCK_DEFAULT_TIMED is asked than a spinning fallback (which could
be implemented externally), but given that we choose to fallback in
previous discussions, but after APR_LOCK_DEFAULT_TIMED was introduced,
maybe we don't need it anymore and it should be axed (we'd still need
a way to ask for the best *timed* mech though...).

View raw message