-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Fix waits on iOS #50388
Fix waits on iOS #50388
Conversation
I've tested with this locally and it seems to fix the issue in simulators. Since there's no CI for iOS right now and my build was a little hacked together, it would be nice if someone else could verify that claim. |
I don't understand why CLOCK_MONOTONIC doesn't work even in newer simulators here, but this will at least fix the problem for now.
06cc184
to
ab482b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I can confirm once packages are available.
I don't recall the exact story (or conclusion) for mono/mono - except that this API has bite us before...
/backport to release/6.0-preview3 |
Started backporting to release/6.0-preview3: https://github.com/dotnet/runtime/actions/runs/701813991 |
The root cause of the problem in dotnet#50388 turned out to be because we do not have `pthread_condattr_setclock(..., CLOCK_MONOTONIC);` on iOS. This caused the timeout argument for `pthread_cond_timedwait` to be interpreted as an _absolute system time_ (in seconds since the Unix epoch), however the value we got back from `clock_gettime(CLOCK_MONOTONIC, ...)` was actually some value based on the uptime of the system. Since the uptime is much smaller than the system time the wait immediately returned. Harden the handling by adding a check for `HAVE_PTHREAD_CONDATTR_SETCLOCK` like we already do in `SystemNative_LowLevelMonitor_Create()`
I did some experiments and found out the root cause: #50441 |
The root cause of the problem in #50388 turned out to be because we do not have `pthread_condattr_setclock(..., CLOCK_MONOTONIC);` on iOS. This caused the timeout argument for `pthread_cond_timedwait` to be interpreted as an _absolute system time_ (in seconds since the Unix epoch), however the value we got back from `clock_gettime(CLOCK_MONOTONIC, ...)` was actually some value based on the uptime of the system. Since the uptime is much smaller than the system time the wait immediately returned. Harden the handling by adding a check for `HAVE_PTHREAD_CONDATTR_SETCLOCK` like we already do in `SystemNative_LowLevelMonitor_Create()`
The root cause of the problem in #50388 turned out to be because we do not have `pthread_condattr_setclock(..., CLOCK_MONOTONIC);` on iOS. This caused the timeout argument for `pthread_cond_timedwait` to be interpreted as an _absolute system time_ (in seconds since the Unix epoch), however the value we got back from `clock_gettime(CLOCK_MONOTONIC, ...)` was actually some value based on the uptime of the system. Since the uptime is much smaller than the system time the wait immediately returned. Harden the handling by adding a check for `HAVE_PTHREAD_CONDATTR_SETCLOCK` like we already do in `SystemNative_LowLevelMonitor_Create()`
The root cause of the problem in #50388 turned out to be because we do not have `pthread_condattr_setclock(..., CLOCK_MONOTONIC);` on iOS. This caused the timeout argument for `pthread_cond_timedwait` to be interpreted as an _absolute system time_ (in seconds since the Unix epoch), however the value we got back from `clock_gettime(CLOCK_MONOTONIC, ...)` was actually some value based on the uptime of the system. Since the uptime is much smaller than the system time the wait immediately returned. Harden the handling by adding a check for `HAVE_PTHREAD_CONDATTR_SETCLOCK` like we already do in `SystemNative_LowLevelMonitor_Create()` Co-authored-by: Alexander Köplinger <[email protected]>
I don't understand why
CLOCK_MONOTONIC
doesn't work even in newer simulators here, but this will at least fix the problem for now.Fixes #50245