Skip to content

Handle different ThreadDestructor signatures between Android NDK 27 and NDK 28 #3249

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

Merged
merged 3 commits into from
May 23, 2025

Conversation

marcprux
Copy link
Contributor

This change enables swift-nio to be compiled for Android against both NDK 27 and NDK 28

Motivation:

Android's NDK had been adopting nullability annotations in its C headers, and they have been changing them from release to release. The churn has mostly died down since the release of the NDK 27 (LTS), but one new change in NDK 28 affects the destructor argument for pthread_create, making the existing ThreadDestructor typealias invalid and leading to the following compile error when the Android SDK bundle is installed and linked to NDK 28:

$ swiftly run swift build --swift-sdk aarch64-unknown-linux-android28 +6.2-snapshot-2025-05-15-a

Building for debugging...
/opt/src/github/swift-everywhere/swift-nio/Sources/NIOPosix/ThreadPosix.swift:82:9: error: cannot convert value of type '@convention(c) (UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer?' to expected argument type '@convention(c) (UnsafeMutableRawPointer) -> UnsafeMutableRawPointer'
 80 |         &handleLinux,
 81 |         nil,
 82 |         destructor,
    |         `- error: cannot convert value of type '@convention(c) (UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer?' to expected argument type '@convention(c) (UnsafeMutableRawPointer) -> UnsafeMutableRawPointer'
 83 |         args
 84 |     )
[4/21] Compiling NIOPosix ThreadPosix.swift

This change adapts the parameter we pass to that function to accommodate both NDK 27 and NDK 28.

Modifications:

There's currently no way to determine which version of the NDK is in use at compile time (see swiftlang/swift#81402), so this accommodation is implementing by having two functions named coerceThreadDestructor that return values that are suitable either NDK 27 or NDK 28 (using unsafeBitCast, but the layout of C functions doesn't know anything about nullability, so this is safe), and then let the type inference for pthread_create handle the rest.

Result:

swift-nio can be compiled against both NDK 27 and NDK 28 without errors.

@glbrntt glbrntt added the 🔨 semver/patch No public API change. label May 19, 2025
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Other than the formatting this LGTM.

diff --git a/Sources/NIOPosix/ThreadPosix.swift b/Sources/NIOPosix/ThreadPosix.swift
index c66da43..69f4fa7 100644
--- a/Sources/NIOPosix/ThreadPosix.swift
+++ b/Sources/NIOPosix/ThreadPosix.swift
@@ -63,13 +63,17 @@ private func sysPthread_create(
     #if os(Android)
     // NDK 27 signature:
     // int pthread_create(pthread_t* _Nonnull __pthread_ptr, pthread_attr_t const* _Nullable __attr, void* _Nonnull (* _Nonnull __start_routine)(void* _Nonnull), void* _Nullable);
-    func coerceThreadDestructor(_ destructor: @escaping ThreadDestructor) -> (@convention(c) (UnsafeMutableRawPointer) -> UnsafeMutableRawPointer) {
+    func coerceThreadDestructor(
+        _ destructor: @escaping ThreadDestructor
+    ) -> (@convention(c) (UnsafeMutableRawPointer) -> UnsafeMutableRawPointer) {
         destructor
     }
 
     // NDK 28 signature:
     // int pthread_create(pthread_t* _Nonnull __pthread_ptr, pthread_attr_t const* _Nullable __attr, void* _Nullable (* _Nonnull __start_routine)(void* _Nullable), void* _Nullable);
-    func coerceThreadDestructor(_ destructor: @escaping ThreadDestructor) -> (@convention(c) (UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer?) {
+    func coerceThreadDestructor(
+        _ destructor: @escaping ThreadDestructor
+    ) -> (@convention(c) (UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer?) {
         unsafeBitCast(destructor, to: (@convention(c) (UnsafeMutableRawPointer?) -> UnsafeMutableRawPointer?).self)
     }
     #else

@marcprux marcprux requested a review from glbrntt May 19, 2025 12:14
@glbrntt glbrntt enabled auto-merge (squash) May 19, 2025 12:25
@Lukasa
Copy link
Contributor

Lukasa commented May 23, 2025

@marcprux Can you either give the admins of this repo write access to this branch, or update the PR to the newest main? Unfortunately, we can't merge PRs that are not up-to-date with main.

@marcprux
Copy link
Contributor Author

I've updated. The usual "Allow edits by maintainers" checkbox doesn't show up for me on this PR, possibly because of this issue. So just LMK if you need it updated again.

@glbrntt glbrntt merged commit b23163d into apple:main May 23, 2025
36 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants