Omnibus to implement CFRunLoop with kevent. #3004

3405691582 · 2021-07-09T23:35:56Z

Dispatch uses kevent for its event loop backend. However, CFRunLoop in Foundation uses Mach ports. This means for other kevent-based platforms such as OpenBSD, new runloop functionality is required, which this omnibus pr implements.

This pr must be considered in concert with swiftlang/swift-corelibs-libdispatch#559 (see below).

Please feel free to request any or all to be split into multiple prs, though all these commits and the aforementioned Dispatch pr are required to properly implement CFRunLoop on this platform.

Notes:

Some of the Mach-based implementation refers to MACH_PORT_NULL, for example, when we have symbols already defined like CFPORT_NULL. These are brought more into alignment for consistency.
Each platform necessitates implementing the "backend" set of __CFPort* and mk_timer_* stubs. It does not make sense if these methods are not implemented for a given platform, so add a default #error case. (The code would greatly benefit from reorganizing the necessary stubs to implement together in one #if conditional, but this should probably be done separate from this omnibus as it is not critically important to achieve our result. However, we have implemented all the stubs together below in this commit.)
A tentative kevent-based implementation of these stubs are provided. In Support OpenBSD. swift-corelibs-libdispatch#559, we introduced a runloop implementation using pipe2 and packing each 32-bit fd in the pipe into a 64-bt integer. Since CFRunLoop fraternizes with Dispatch via _dispatch_main_queue_callback_4CF, this means that dispatch_runloop_handle_t in Dispatch must match __CFPort here. Unfortunately, this means that we have to perform some bit-packing shenanigans to handle kevent timers. There might be nicer ways to do this, but we'll defer that for now.
We need to fix SR-14288 while we are here to ensure the runloop timers work correctly and the tests pass properly.
While implementing the runloop, testing has uncovered some strange behavior with pthread_main_np and TSD on this platform. Work around this by doing a simple pthread_self comparison.

3405691582 · 2021-07-17T05:45:43Z

cc @millenomi -- input or advice on how to proceed?

AZero13 · 2021-10-04T14:45:57Z

@compnerd any thoughts?

finagolfin · 2021-12-10T19:36:13Z

@parkera, mind reviewing?

finagolfin · 2022-07-05T19:09:19Z

@phausler, maybe you could review?

CFRunLoop has a recurring abstraction for platform-specific code, but it is a little leaky. Plug these leaks: ensure `MACH_PORT_NULL` in the generic, non-platform context is rewritten as `CFPORT_NULL`, and ensure that `kern_return_t` and `KERN_SUCCESS` are defined properly.

New platforms must define these types and functions; it is incorrect to not have #else cases here.

finagolfin · 2022-07-25T14:54:34Z

@etcwilde, would you review?

etcwilde

This looks fine for the most part. I left a couple comments on style.

I'm unfortunately not familiar enough with this part of the codebase to really identify logic/behavior issues in the implementation of the __CFPort* or mk_timer function implementations down in CFRunLoop.c. It looks fine from what I can tell, but it would be better to get someone more familiar with this code to look over it and verify that it's correct.

As I'm not very familiar with this part of the codebase, nor am I an owner or maintainer of foundation, I don't really feel comfortable just approving it. I'll leave that part up to @parkera.

etcwilde · 2022-07-29T06:09:25Z

CoreFoundation/RunLoop.subproj/CFRunLoop.c

+static inline uint64_t __CFNanosecondsToTSR(uint64_t ns) {
+#if TARGET_OS_MAC || TARGET_OS_LINUX
+    return ns;
+#else


@compnerd, is this the appropriate implementation of this function for Windows?

etcwilde · 2022-07-29T06:13:57Z

CoreFoundation/Base.subproj/CFPlatform.c

@@ -249,7 +249,11 @@ const char *_CFProcessPath(void) {

 #if TARGET_OS_MAC || TARGET_OS_WIN32 || TARGET_OS_BSD
 CF_CROSS_PLATFORM_EXPORT Boolean _CFIsMainThread(void) {
+#if !defined(__OpenBSD__)


It's a bit clearer if we have the OpenBSD case come first and not invert the condition.

#if defined(__OpenBSD__) // special case for OpenBSD return pthread_equal(pthread_self(), _CFMainPThread) != 0; #else // everything else return pthread_main_np() == 1; #endif

I usually think of else indicating anything not special-cased, so having the else only catch the OpenBSD case seems a bit odd.
Thoughts?

Reordered the conditional.

etcwilde · 2022-07-29T06:22:22Z

CoreFoundation/RunLoop.subproj/CFRunLoop.c

+    EV_SET(
+        &tev,
+      id,
+      EVFILT_TIMER,


Inconsistent indentation through here.

Fixes SR-14288.

During runloop testing, log messages suggest that the runloop thinks the main thread has exited and certain runloop operations don't actually proceed. The flag for denoting when the main thread has exited is set in the thread-specific data destructor `__CFTSDFinalize`. This function detects whether the thread in question is the main thread via `pthread_main_np`, and has been observed in debugging this problem to return 1 on apparently non-main threads on OpenBSD. This obviously will cause the main thread exited flag to be erroneously set when non-main threads complete their work and dispose of thread-specific data. Instead, use the existing `_CFMainPThread` symbol set in `__CFInitialize` to determine the main thread. I am not sure whether the platform `pthread_main_np` is at fault here, but since this workaround makes the tests pass, let's use it.

parkera

Looks good to me.

parkera · 2022-08-01T16:05:06Z

@swift-ci test

compnerd · 2022-08-09T22:25:16Z

I think that this might be causing the timeout on Windows ...

This reverts commit 5f57e70.

* [runloop] Set up some symbols for generic code. CFRunLoop has a recurring abstraction for platform-specific code, but it is a little leaky. Plug these leaks: ensure `MACH_PORT_NULL` in the generic, non-platform context is rewritten as `CFPORT_NULL`, and ensure that `kern_return_t` and `KERN_SUCCESS` are defined properly. * [runloop] Remind porter these stubs are required. New platforms must define these types and functions; it is incorrect to not have #else cases here. * [runloop] Implement runloop abstraction for BSD. * [runloop] Avoid pthread_main_np on OpenBSD. During runloop testing, log messages suggest that the runloop thinks the main thread has exited and certain runloop operations don't actually proceed. The flag for denoting when the main thread has exited is set in the thread-specific data destructor `__CFTSDFinalize`. This function detects whether the thread in question is the main thread via `pthread_main_np`, and has been observed in debugging this problem to return 1 on apparently non-main threads on OpenBSD. This obviously will cause the main thread exited flag to be erroneously set when non-main threads complete their work and dispose of thread-specific data. Instead, use the existing `_CFMainPThread` symbol set in `__CFInitialize` to determine the main thread. I am not sure whether the platform `pthread_main_np` is at fault here, but since this workaround makes the tests pass, let's use it.

Omnibus to implement CFRunLoop with kevent. [runloop] Set up some symbols for generic code. CFRunLoop has a recurring abstraction for platform-specific code, but it is a little leaky. Plug these leaks: ensure `MACH_PORT_NULL` in the generic, non-platform context is rewritten as `CFPORT_NULL`, and ensure that `kern_return_t` and `KERN_SUCCESS` are defined properly. [runloop] Remind porter these stubs are required. New platforms must define these types and functions; it is incorrect to not have #else cases here. [runloop] Implement runloop abstraction for BSD. [runloop] Ensure timing arithmetic is done in ns. Fixes SR-14288. [runloop] Avoid pthread_main_np on OpenBSD. During runloop testing, log messages suggest that the runloop thinks the main thread has exited and certain runloop operations don't actually proceed. The flag for denoting when the main thread has exited is set in the thread-specific data destructor `__CFTSDFinalize`. This function detects whether the thread in question is the main thread via `pthread_main_np`, and has been observed in debugging this problem to return 1 on apparently non-main threads on OpenBSD. This obviously will cause the main thread exited flag to be erroneously set when non-main threads complete their work and dispose of thread-specific data. Instead, use the existing `_CFMainPThread` symbol set in `__CFInitialize` to determine the main thread. I am not sure whether the platform `pthread_main_np` is at fault here, but since this workaround makes the tests pass, let's use it.

3405691582 mentioned this pull request Jul 23, 2021

Support OpenBSD. swiftlang/swift-corelibs-libdispatch#559

Merged

3405691582 force-pushed the RunLoopWithKevent branch from 26ac266 to 757836f Compare September 10, 2021 21:58

3405691582 mentioned this pull request Dec 14, 2021

[stdlib] Use the single-header Glibc modulemap. swiftlang/swift#40557

Merged

3405691582 added 2 commits July 18, 2022 17:45

[runloop] Remind porter these stubs are required.

386e107

New platforms must define these types and functions; it is incorrect to not have #else cases here.

3405691582 force-pushed the RunLoopWithKevent branch from 757836f to a36dd52 Compare July 18, 2022 21:45

etcwilde reviewed Jul 29, 2022

View reviewed changes

3405691582 added 3 commits July 30, 2022 18:25

[runloop] Implement runloop abstraction for BSD.

ca0f193

[runloop] Ensure timing arithmetic is done in ns.

05ceecf

Fixes SR-14288.

3405691582 force-pushed the RunLoopWithKevent branch from a36dd52 to 13bc94f Compare July 30, 2022 22:28

parkera approved these changes Aug 1, 2022

View reviewed changes

parkera merged commit 5f57e70 into swiftlang:main Aug 2, 2022

3405691582 mentioned this pull request Aug 6, 2022

[SR-14288] CFRunLoop assumes mach_absolute_time() is in TSR units #4220

Open

compnerd added a commit that referenced this pull request Aug 9, 2022

Revert "Omnibus to implement CFRunLoop with kevent. (#3004)"

cf9d303

This reverts commit 5f57e70.

compnerd mentioned this pull request Aug 9, 2022

Revert "Omnibus to implement CFRunLoop with kevent." #4621

Closed

3405691582 mentioned this pull request Aug 9, 2022

Attempt to unbreak test timeouts in Windows. #4622

Merged

lin72h mentioned this pull request Aug 18, 2022

Swift build on FreeBSD ravynsoft/ravynos#209

Open

3405691582 mentioned this pull request Oct 19, 2022

The Ventura Core Foundation Merge #4633

Closed

eleanxr mentioned this pull request Oct 21, 2022

Use CoreFoundation from Swift? ravynsoft/ravynos#82

Open

eleanxr mentioned this pull request Nov 12, 2022

Use CoreFoundation from Swift ravynsoft/ravynos#381

Closed

michael-yuji mentioned this pull request Mar 17, 2025

[FreeBSD] support FreeBSD swiftlang/swift-corelibs-libdispatch#861

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Omnibus to implement CFRunLoop with kevent. #3004

Omnibus to implement CFRunLoop with kevent. #3004

3405691582 commented Jul 9, 2021 •

edited

Loading

3405691582 commented Jul 17, 2021

AZero13 commented Oct 4, 2021

finagolfin commented Dec 10, 2021

finagolfin commented Jul 5, 2022

finagolfin commented Jul 25, 2022

etcwilde left a comment

etcwilde Jul 29, 2022

etcwilde Jul 29, 2022

3405691582 Jul 30, 2022

etcwilde Jul 29, 2022

3405691582 Jul 30, 2022

parkera left a comment

parkera commented Aug 1, 2022

compnerd commented Aug 9, 2022

Omnibus to implement CFRunLoop with kevent. #3004

Omnibus to implement CFRunLoop with kevent. #3004

Conversation

3405691582 commented Jul 9, 2021 • edited Loading

3405691582 commented Jul 17, 2021

AZero13 commented Oct 4, 2021

finagolfin commented Dec 10, 2021

finagolfin commented Jul 5, 2022

finagolfin commented Jul 25, 2022

etcwilde left a comment

Choose a reason for hiding this comment

etcwilde Jul 29, 2022

Choose a reason for hiding this comment

etcwilde Jul 29, 2022

Choose a reason for hiding this comment

3405691582 Jul 30, 2022

Choose a reason for hiding this comment

etcwilde Jul 29, 2022

Choose a reason for hiding this comment

3405691582 Jul 30, 2022

Choose a reason for hiding this comment

parkera left a comment

Choose a reason for hiding this comment

parkera commented Aug 1, 2022

compnerd commented Aug 9, 2022

3405691582 commented Jul 9, 2021 •

edited

Loading