Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Problem: pthread condvar timeouts are broken #2967

Merged
merged 2 commits into from
Mar 5, 2018
Merged

Problem: pthread condvar timeouts are broken #2967

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b5a238c
fix pthread condvar timeouts
lunixbochs Mar 5, 2018
8e84fc4
fix pthread condvar timeouts for android
lunixbochs Mar 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions src/condition_variable.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,26 @@ class condition_variable_t

#include <pthread.h>

#if defined(__ANDROID_API__) && __ANDROID_API__ < 21
#define ANDROID_LEGACY
extern "C" int pthread_cond_timedwait_monotonic_np (pthread_cond_t *,
pthread_mutex_t *,
const struct timespec *);
#endif

namespace zmq
{
class condition_variable_t
{
public:
inline condition_variable_t ()
{
int rc = pthread_cond_init (&cond, NULL);
pthread_condattr_t attr;
pthread_condattr_init (&attr);
#if !defined(ZMQ_HAVE_OSX) && !defined(ANDROID_LEGACY)
pthread_condattr_setclock (&attr, CLOCK_MONOTONIC);
#endif
int rc = pthread_cond_init (&cond, &attr);
posix_assert (rc);
}

Expand All @@ -198,9 +210,9 @@ class condition_variable_t
if (timeout_ != -1) {
struct timespec timeout;

#if defined ZMQ_HAVE_OSX \
&& __MAC_OS_X_VERSION_MIN_REQUIRED < 101200 // less than macOS 10.12
alt_clock_gettime (SYSTEM_CLOCK, &timeout);
#ifdef ZMQ_HAVE_OSX
timeout.tv_sec = 0;
timeout.tv_nsec = 0;
#else
clock_gettime (CLOCK_MONOTONIC, &timeout);
#endif
Expand All @@ -212,8 +224,15 @@ class condition_variable_t
timeout.tv_sec++;
timeout.tv_nsec -= 1000000000;
}

#ifdef ZMQ_HAVE_OSX
rc = pthread_cond_timedwait_relative_np (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this available on all versions of OSX? There have been many issues with OSX and clocks over the years, see:
#2537
#2538
#1351
#1354

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found pthread_cond_timedwait_relative_np recommended at least 6 years ago for macOS.

For Android, looks like that was the wrong check, and I should just be checking for r21 or not:
https://stackoverflow.com/questions/44580542/ndk-builder-r15-finds-neither-have-pthread-cond-timedwait-monotonic-nor-pthread

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's referenced in https://opensource.apple.com/source/Libc/Libc-166/pthreads.subproj/pthread_cond.c, which is literally OS X 10.0. So yeah I'd say it's probably fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for checking

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android build fixed. The clang format build is still erroring, but not on my changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for your contribution, merged

&cond, mutex_->get_mutex (), &timeout);
#elif defined(ANDROID_LEGACY)
rc = pthread_cond_timedwait_monotonic_np (
&cond, mutex_->get_mutex (), &timeout);
#else
rc = pthread_cond_timedwait (&cond, mutex_->get_mutex (), &timeout);
#endif
} else
rc = pthread_cond_wait (&cond, mutex_->get_mutex ());

Expand Down