-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Time.cpp's clock_gettime fallback breaks libc++ steady_clock::now #1602
Comments
rcari
added a commit
to rcari/folly
that referenced
this issue
Feb 25, 2022
The clock_gettime situation on Apple platforms is complex enough as it is. Linking an incomplete implementation in replacement of the global symbol breaks other libraries including libc++ which uses clock_gettime internally [1]. Depending on the approach taken by the libc++ the current implementation could trigger infinite recursion calls as well. Since the goal of the /portability/ is to make Folly portable, we should adapt and use an approach which is compatible with Apple SDK development while maintaining compatibility on other platforms. This patch does that by adding a folly_ prefix to clock functions to avoid clashing with the native symbol. It's the role of other libraries to adhere to Apple SDK development guidelines when using the clock_ family of functions. As a side effect, this also addresses the redefinition issues that this [2] other proposed fix solved. [1] facebook#1602 [2] facebook#1724
I proposed #1725 to get rid of this issue along with some others @yfeldblum |
rcari
added a commit
to rcari/folly
that referenced
this issue
Aug 5, 2022
The clock_gettime situation on Apple platforms is complex enough as it is. Linking an incomplete implementation in replacement of the global symbol breaks other libraries including libc++ which uses clock_gettime internally [1]. Depending on the approach taken by the libc++ the current implementation could trigger infinite recursion calls as well. Since the goal of the /portability/ is to make Folly portable, we should adapt and use an approach which is compatible with Apple SDK development while maintaining compatibility on other platforms. This patch does that by adding a folly_ prefix to clock functions to avoid clashing with the native symbol and #define clock_getX folly_clock_getX in order to maintain source compatibility. As a side effect, this also addresses the redefinition issues that this [2] other proposed fix solved. [1] facebook#1602 [2] facebook#1724
rcari
added a commit
to rcari/folly
that referenced
this issue
Aug 5, 2022
The clock_gettime situation on Apple platforms is complex enough as it is. Linking an incomplete implementation in replacement of the global symbol breaks other libraries including libc++ which uses clock_gettime internally [1]. Depending on the approach taken by the libc++ the current implementation could trigger infinite recursion calls as well. Since the goal of the /portability/ is to make Folly portable, we should adapt and use an approach which is compatible with Apple SDK development while maintaining compatibility on other platforms. This patch does that by adding a folly_ prefix to clock functions to avoid clashing with the native symbol and #define clock_getX folly_clock_getX in order to maintain source compatibility. As a side effect, this also addresses the redefinition issues that this [2] other proposed fix solved. [1] facebook#1602 [2] facebook#1724
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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++ callsclock_gettime(CLOCK_MONOTONIC_RAW, &tp)
on Apple platforms.Our users have been reporting that
std::steady_clock::now()
throws after callingclock_gettime
, resulting in an uncaught exception and a crash: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++'sclock_gettime
import, even on devices >= iOS 10 where the symbol would otherwise be loaded fromlibsystem_c.dylib
at runtime.That would be fine, except Folly's clock_gettime does not support CLOCK_MONOTONIC_RAW. 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). (This can also happen if a dep is still using an older version of Folly where the macro was accidentally always true in some cases, but presumably these deps will update eventually).
Note that this is a different issue from #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:
clock_gettime
fallback supports less features than the native Apple implementation, and in particular does not supportCLOCK_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 ofclock_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:clock_gettime
polyfill to other dependencies. It seems there was a patch at some point that namespaced the symbol, though it was quickly reverted. There may be some other way to achieve the same result.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.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!
The text was updated successfully, but these errors were encountered: