-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[Android] Put Java JNI function in a separate static library #108513
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
base: main
Are you sure you want to change the base?
[Android] Put Java JNI function in a separate static library #108513
Conversation
aa4fac2
to
7e67d99
Compare
Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger |
c77e32e
to
25d242e
Compare
25d242e
to
9521f00
Compare
@grendello I'm not sure what state this is in. I've been assuming since October that someone else from .NET Android was going to sign off... it's outside of my domain. But since it's in my area and I'm trying to get things tidied up... "do we want this, or should we close the PR?" |
9521f00
to
d8003f8
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.
Pull Request Overview
This PR refactors the Android cryptography native library to support dynamic linking in .NET for Android runtime by separating a Java JNI function into its own static library. The key motivation is to enable symbol hiding for all BCL native library exports while keeping the JNI function visible to the Java Virtual Machine.
- Move JNI function to separate static library to control symbol visibility during dynamic linking
- Refactor callback storage mechanism to support the new architecture
- Update build configuration to include the new static library in all relevant targets
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
pal_trust_manager_jni_export.c | New file containing the JNI function and atomic callback storage |
pal_trust_manager.h | Add function declaration for the new callback storage function |
pal_trust_manager.c | Remove JNI function and atomic storage, delegate to new storage function |
CMakeLists.txt | Add new static library target for JNI exports with detailed comments |
apphost/static/CMakeLists.txt | Include new static library in native libs list |
Directory.Build.props | Add new static library to platform manifest |
Comments suppressed due to low confidence (1)
src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c:6
- The function name 'StoreRemoteVerificationCallback' doesn't follow the established naming convention. Based on the existing code pattern, it should be prefixed with 'AndroidCryptoNative_' like other public functions in this module.
void StoreRemoteVerificationCallback (RemoteCertificateValidationCallback callback)
src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c
Show resolved
Hide resolved
d8003f8
to
436d952
Compare
I am not necessarily opposed to it but this feels like a pretty heavy solution. In case of NativeAOT it's possible to fix this with just including this in MSBuild item group: <IlcArg Include="--export-dynamic-symbol:Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate" /> The build process generates an exports file which is then fed to the linker. I wonder if the CoreCLR/Android build process uses something similar because the linker script takes a precedence over For reference, the NativeAOT exports file looks like this:
|
@filipnavara I played with the option you mention, with varying success. The goal I have in mind is to hide symbols by default with few exceptions, and for static linking this is easier than playing with |
Have you verified that the new symbol gets exported from the AOT compiled binary? I do not see how it can get exported there. |
src/native/libs/System.Security.Cryptography.Native.Android/pal_trust_manager_jni_export.c
Show resolved
Hide resolved
It's not... well, at least not in all modes / by default. There are two modes of compilation. |
When linking statically a NativeAOT app, does it really need to export the symbol though? NativeAOT is all native, there are no BCL assemblies which p/invoke that symbol. Instead, everything is referenced locally at link time. Android has to export it, because we still use actual managed assemblies. |
Yes, it does. It's a JNI interface and the JavaVM/Dalvik looks for it. |
Yeah, you're right. I forgot it is actually called from Java. |
Eventually, I think the runtime pack should provide a manifest of libraries that are to be used for linking as well as the init functions to call (if any) and what symbols to export. For now, I'd say responsibility to export this (and potentially others) symbol rests on the consumer (NativeAOT or Android in this case) |
cadc93e
to
4dc1629
Compare
I agree - the export should be next to logic that references the library. Add |
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
Outdated
Show resolved
Hide resolved
src/native/libs/System.Security.Cryptography.Native.Android/CMakeLists.txt
Outdated
Show resolved
Hide resolved
In dotnet/android#9006 we are working on linking the .NET for Android runtime dynamically at the application build time. Linking involves all the BCL native libraries, including `System.Security.Cryptography.Native.Android` and one of the goals is to hide all the exported symbols used as p/invokes by the managed BCL libraries. This is because p/invoke calls are handled internally and, with dynamic linking of the runtime, there is no longer any reason to use `dlopen` and `dlsym` to look them up, they are all resolved internally, at the link time. Symbol hiding works fine thanks to the `--exclude-libs` `clang` flag, which makes all the exported symbols in the indicated `.a` archives to not be exported by the linker. However, `System.Security.Cryptography.Native.Android` is special in the sense that it contains one symbol which must not be hidden, `Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate`. The above function is a Java `native` method implementation, and it requires that not only its name follows the Java JNI naming rules, but that is also available for the JVM to look up using `dlsym`. I tried using the `--export-dynamic-symbol` clang flag to export **just** this function, but it doesn't appear to work no matter where I put the flag in relation to reference to the `.a` archives. Instead, the problem can be dealt with by putting the JNI function in a separate static library, so that I can link it without changing symbol visibility, while making all the `System.Security.Cryptography.Native.Android` symbols invisible.
…e.Unix.targets Co-authored-by: Filip Navara <[email protected]>
…e.Unix.targets Co-authored-by: Jan Kotas <[email protected]>
a17572a
to
90dbd76
Compare
In dotnet/android#9006 we are working on linking the
.NET for Android
runtime dynamically at the application build time.
Linking includes all the relevant BCL native libraries, and one of the goals is to
hide all the exported symbols used as
p/invokes
by the managed BCLlibraries. This is because
p/invoke
calls are handled internally and,with dynamic linking of the runtime, there is no longer any reason to
use
dlopen
anddlsym
to look them up, they are all resolvedinternally, at the link time.
Symbol hiding works fine thanks to the
--exclude-libs
clang
flag,which makes all the exported symbols in the indicated
.a
archives tonot be exported by the linker. However,
System.Security.Cryptography.Native.Android
is special in the sense that it contains one symbol which must not be hidden,
Java_net_dot_android_crypto_DotnetProxyTrustManager_verifyRemoteCertificate
.The above function is a Java
native
method implementation, and itrequires that not only its name follows the Java JNI naming rules, but
that it is also available for the JVM to look up using
dlsym
.I tried using the
--export-dynamic-symbol
clang flag toexport just this function, but it doesn't appear to work no matter
where I put the flag in relation to reference to the
.a
archives.Instead, the problem can be dealt with by putting the JNI function in a
separate static library, so that I can link it without changing symbol
visibility, while making all the
System.Security.Cryptography.Native.Android
symbols invisible.