legacy TIMEDWAIT declarations missing from Unified Headers

[alexcohn]

Description

This surfaced in WebRTC compilation on r15 (see e.g. event_timer_posix.c:99).

platform headers for android-9 till android-19 define HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC. It is not defined in unified headers, but simply adding this with condition __ANDROID_API__ < 21 does not help; we also need to declare pthread_cond_timedwait_monotonic_np() and her kin.

Workaround:

I have added the file pthread.h high on our includes search path:

#include_next <pthread.h>

// workaround a bug with Unified Headers in NDK r.15 (see https:/android-ndk/ndk/issues/423)

#if defined __ANDROID_API__ && (__ANDROID_API__ < 21)

#ifdef __cplusplus
extern "C" {
#endif

/* BIONIC: same as pthread_cond_timedwait, except the 'abstime' given refers
 *         to the CLOCK_MONOTONIC clock instead, to avoid any problems when
 *         the wall-clock time is changed brutally
 */
int pthread_cond_timedwait_monotonic_np(pthread_cond_t         *cond,
                                        pthread_mutex_t        *mutex,
                                        const struct timespec  *abstime);

/* BIONIC: DEPRECATED. same as pthread_cond_timedwait_monotonic_np()
 * unfortunately pthread_cond_timedwait_monotonic has shipped already
 */
int pthread_cond_timedwait_monotonic(pthread_cond_t         *cond,
                                     pthread_mutex_t        *mutex,
                                     const struct timespec  *abstime);

#define HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC 1

/* BIONIC: same as pthread_cond_timedwait, except the 'reltime' given refers
 *         is relative to the current time.
 */
int pthread_cond_timedwait_relative_np(pthread_cond_t         *cond,
                                     pthread_mutex_t        *mutex,
                                     const struct timespec  *reltime);

#define HAVE_PTHREAD_COND_TIMEDWAIT_RELATIVE 1



int pthread_cond_timeout_np(pthread_cond_t *cond,
                            pthread_mutex_t * mutex,
                            unsigned msecs);

/* same as pthread_mutex_lock(), but will wait up to 'msecs' milli-seconds
 * before returning. same return values than pthread_mutex_trylock though, i.e.
 * returns EBUSY if the lock could not be acquired after the timeout
 * expired.
 */
int pthread_mutex_lock_timeout_np(pthread_mutex_t *mutex, unsigned msecs);

#ifdef __cplusplus
}
#endif

#endif

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

• NDK Version: 15.0.4075724.
• Build system: ndk-build
• Host OS: Mac
• Compiler: Clang
• ABI: armeabi-v7a and others
• STL: -
• NDK API level: android-14
• Device API level: n/a

[alexcohn]

actually, it's duplicate of #420

[alexcohn]

@DanAlbert did you set 'invalid' because it's duplicate?

[DanAlbert]

Yep, my mistake.

[DanAlbert]

Crap, I actually missed the list of other missing things. I'm going to get these added now.

[DanAlbert]

https://android-review.googlesource.com/c/420945/ sorry about that!

[DanAlbert]

Oh, I didn't put back the HAVE_BLAH defines though. I don't want us to overlap with autoconf and cause macro redefinition warnings. Those checks can be written in terms of __ANDROID_API__ for people not using configure.

[alexcohn]

@DanAlbert Do you expect people to migrate their code, e.g. WebRTC to use

#if (__ANDROID_API__ < 21)

in event_timer_posix.cc:99? I would rather keep it the way it is today (anyway, WebRTC still ships with NDK r.11). People who will try to use autoconf may need some adaptations, but at least don't force projects that are tuned for NDK already to change: their natural path will be to stay stuck with old NDK as long as possible.

[enh]

as far as we're concerned, the sooner everyone's on the same codepath, the better. the pthread_cond_timedwait codepath they need for LP64 works fine for LP32 too. for us, failing the HAVE_BLAH check seems like a feature.

[DanAlbert]

The fallback behavior actually doesn't work pre-21, because we didn't have pthread_condattr_setclock until 21, so the clock won't be monotonic.

[enh]

yeah, i was thinking of HAVE_PTHREAD_COND_TIMEDWAIT_RELATIVE. now you're making me distinguish the two cases, yes, the argument for HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC is much stronger because pthread_condattr_setclock was a late addition.

[alexcohn]

This fix missed

#define HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC 1

and thus WebRTC breaks for webrtc/system_wrappers/source/event_timer_posix.cc when built with ANDROID_API < 21.

[enh]

yeah, polluting the global namespace with HAVE_ macros seems unreasonable.

[alexcohn]

@enh, do you suggest to open a ticket for WebRTC to switch this check to __ANDROID_API__ < 21 ?

[enh]

yeah, that sounds like the right fix.

[alexcohn]

They have chosen to define HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC, see https://chromium.googlesource.com/chromium/src/build/+/master/config/android/BUILD.gn#96

They have a bug for sdk level=20, but it's purely theoretical.