Hi Bill, On Thu, Jun 1, 2017 at 6:32 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote: > On Mon, May 22, 2017 at 3:22 PM, Yann Ylavic <ylavic....@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...).
