Time.cpp's clock_gettime fallback breaks libc++ steady_clock::now
- Dominant language
- C++
- Stars
- 30.5k
- Forks
- 5.9k
- PR merge metrics
- No merged PRs in 30d
Description
Hello,
We maintain a Native Module for React-Native that supports Apple platforms, and so we (and our users) have a dependency on Folly through the RN ecosystem.
Our module statically links with libc++, and some of our own C++ dependencies use `std::steady_clock`, which in recent versions of libc++ [calls `clock_gettime(CLOCK_MONOTONIC_RAW, &tp)`](https://github.com/llvm/llvm-project/blob/llvmorg-11.1.0/libcxx/src/chrono.cpp#L123-L129) on Apple platforms.
Our users have been reporting that `std::steady_clock::now()` throws after calling `clock_gettime`, resulting in an uncaught exception and a crash:
```cpp
* thread #23, stop reason = signal SIGABRT
* frame #0: 0x00007fff6113133a libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x00007fff61166e60 libsystem_pthread.dylib`pthread_kill + 430
frame #2: 0x00007fff200fab94 libsystem_c.dylib`abort + 120
frame #3: 0x000000010da81ff3 ExampleApp`abort_message + 195
frame #4: 0x000000010d878fae ExampleApp`demangling_terminate_handler() + 238
frame #5: 0x000000010d92f0f8 ExampleApp`std::__terminate(void (*)()) + 8
frame #6: 0x000000010d92f079 ExampleApp`std::terminate() + 41
frame #7: 0x000000010d85dfbb ExampleApp`__clang_call_terminate + 11
frame #8: 0x000000010e04bdf9 ExampleApp`std::__1::chrono::steady_clock::now() + 73
```
## Why do things break
It turns out that Folly's fallback implementation exports its `clock_gettime` implementation globally, as a weak symbol! So when our native module is statically linked into the user's application, Folly's clock_gettime symbol will bind to libc++'s `clock_gettime` import, even on devices >= iOS 10 where the symbol would otherwise be loaded from `libsystem_c.dylib` at runtime.
That would be fine, except Folly's clock_gettime [does not support CLOCK_MONOTONIC_RAW](https://github.com/facebook/folly/blob/master/folly/portability/Time.cpp#L117-L119). It returns EINVAL, which causes steady_clock to throw, and our users get to keep both pieces. This is suboptimal :(
## The macro in Time.h
The React-Native build system can be *fairly* complex, and as a native module we don't have control on all the other dependencies our users may have and on how Folly is compiled by those deps. However it is our understanding that the clock_gettime fallback can end up getting pulled in either when a dep supports iOS platforms < 10 (which triggers [the macro in Time.h](https://github.com/facebook/folly/blame/master/folly/portability/Time.h)). (This can also happen if a dep is [still using an older version](https://github.com/CocoaPods/Specs/blob/master/Specs/5/7/2/Flipper-Folly/2.6.7/Flipper-Folly.podspec.json#L12) of Folly where the macro was [accidentally always true](https://github.com/facebook/folly/commit/c85f8af60e912ab09bdf00153a65bef5ab9b2204) in some cases, but presumably these deps will update eventually).
Note that this is a different issue from https://github.com/facebook/folly/issues/1470, which results in breakage when that macro is false. Here things break when the macro is true.
## Expected behavior
This incompatibility is caused by a combination of two things:
- If anyone in a React-Native user's dependency tree imports Folly with support for iOS 9, the symbol will be global and affects all other dependencies, outside of Folly.
- The `clock_gettime` fallback supports less features than the native Apple implementation, and in particular does not support `CLOCK_MONOTONIC_RAW`, which is required by libc++ on Apple platforms.
We are currently working around the bug by using a patched version of libc++ that unconditionally calls `mach_absolute_time` instead of `clock_gettime`, even when our target platforms have this support, however considering the above I think the bug can be fixed in one of three ways:
- Ideally, Folly should not expose its `clock_gettime` polyfill to other dependencies. It seems there was a patch at some point that namespaced the symbol, though [it was quickly reverted](https://github.com/facebook/folly/commit/5de2b2828599520c64bfb9749259c796f074df59). There may be some other way to achieve the same result.
- Consider dropping the compatibility macros entirely. It seems the `clock_gettime` situation is complicated enough on Apple platforms that by trying to support iOS 9 and lower, the macro is causing *more* incompatibilities for newer versions.
- If Folly exports a polyfill globally, it should support `CLOCK_MONOTONIC_RAW`, which according to libc++ is "the only acceptable implementation of steady_clock".
Any one of these would fix this issue, though I think we would be happy with any solution that resolves the incompatibility between React-Native libraries that use Folly and libc++.
Thank you!
Contributor guide
Assessment
This issue has not been assessed yet.