Skip to content
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

Fix loadLibraries() failing for 64bit arch #1701

Merged
merged 1 commit into from
Feb 25, 2019

Conversation

j-devel
Copy link
Contributor

@j-devel j-devel commented Feb 11, 2019

loadLibraries() fails in loading native libs when arch is arm64-v8a or x86_64.

Explanation

On 64-bit Android, the path obtained by getFilesDir().getParentFile().getAbsolutePath() + "/lib" (e.g. /data/data/<package>/lib) is not available (ref.; I did verify this on my 64-bit Android devices). For the same reason, libsDirPath in getLibraries() below is resolved to a non-existent path:

String libsDirPath = filesDir.getParentFile().getParentFile().getAbsolutePath() + "/lib/";
File libsDir = new File(libsDirPath);
ArrayList<String> libsList = new ArrayList<String>();
addLibraryIfExists(libsList, "crystax", libsDir);

As a result, in addLibraryIfExists(), a NullPointerException is thrown at files.length. (Note that listFiles() returns null for an invalid directory path per this ref.)

File [] files = libsDir.listFiles();
pattern = "lib" + pattern + "\\.so";
Pattern p = Pattern.compile(pattern);
for (int i = 0; i < files.length; ++i) {

Although this exception is being caught, native libs (libSDL.so, libmain.so, etc.) are not loaded, so Android crashes anyway:

02-10 21:16:01.635 17125 17125 W System.err: Attempt to get length of null array
02-10 21:16:01.667 17125 17125 E art     : No implementation found for void org.libsdl.app.SDLActivity.nativeSetenv(java.lang.String, java.lang.String) (tried Java_org_libsdl_app_SDLActivity_nativeSetenv and Java_org_libsdl_app_SDLActivity_nativeSetenv__Ljava_lang_String_2Ljava_lang_String_2)
02-10 21:16:01.667 17125 17125 D AndroidRuntime: Shutting down VM
02-10 21:16:01.668 17125 17125 E AndroidRuntime: FATAL EXCEPTION: main

Fix

Use getApplicationInfo().nativeLibraryDir instead, which is officially documented.

@AndreMiras
Copy link
Member

Nice investigation and thank you for the PR. I guess we ne to test it at least on ARMv7 to see it didn't introduce a regression

@j-devel
Copy link
Contributor Author

j-devel commented Feb 15, 2019

👍 On armv7, I haven't seen anything odd so far due to the changes, and I will inform in case I notice anything while using my builds.

@OptimusGREEN
Copy link
Contributor

I've tested this with arm 7 and 8 and both apps ran without issue that I could see. it also fixed #1719 on arm 8 for me.

Copy link
Contributor

@OptimusGREEN OptimusGREEN left a comment

Choose a reason for hiding this comment

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

This fixes arm v8 builds for me and have seen no detrimental effects in arm 7a after several successful builds.

@inclement
Copy link
Member

Thanks, and also to @OptimusGREEN for the testing.
I wonder if this will also require fixing any other references to that directory, such as for @Jonast's ctypes patch, although probably nothing major.

@inclement inclement merged commit 5d0f92e into kivy:master Feb 25, 2019
@ghost
Copy link

ghost commented Feb 26, 2019

@inclement yeah I would assume both the original python 2 ctypes patch (not my work) and my rip-off variant for python 3 would break in armv8. since pyjnius doesn't depend on ctypes, the best approach I can think of is to use that to access this java function inside our ctypes patch

@j-devel j-devel deleted the fix-loadLibraries-64bit-arch branch February 26, 2019 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants