Skip to content

Commit 93e4028

Browse files
committed
[WIP] workaround for System.loadLibrary not working on some threads
1 parent d74f72b commit 93e4028

File tree

10 files changed

+213
-28
lines changed

10 files changed

+213
-28
lines changed

src/native/clr/host/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ macro(lib_target_options TARGET_NAME)
154154
xa::java-interop
155155
xa::pinvoke-override-precompiled
156156
xa::lz4
157+
-landroid
157158
-llog
158159
-lcoreclr
159160
)

src/native/clr/host/dso-loader.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#include <runtime-base/dso-loader.hh>
2-
32
#include <host/os-bridge.hh>
43

54
using namespace xamarin::android;

src/native/clr/host/host.cc

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
#include <cerrno>
55
#include <cstdio>
66
#include <cstring>
7+
#include <unistd.h>
8+
9+
#include <android/looper.h>
710

811
#include <coreclrhost.h>
912

@@ -422,7 +425,12 @@ void Host::Java_mono_android_Runtime_initInternal (
422425
}
423426
);
424427

425-
DsoLoader::init (env, RuntimeUtil::get_class_from_runtime_field (env, runtimeClass, "java_lang_System", true));
428+
DsoLoader::init (
429+
env,
430+
RuntimeUtil::get_class_from_runtime_field (env, runtimeClass, "java_lang_System", true),
431+
ALooper_forThread (), // main thread looper
432+
gettid ()
433+
);
426434

427435
struct JnienvInitializeArgs init = {};
428436
init.javaVm = jvm;

src/native/common/include/runtime-base/dso-loader.hh

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,16 @@
22

33
#include <jni.h>
44
#include <dlfcn.h>
5+
#include <unistd.h>
6+
57
#include <android/dlext.h>
8+
#include <android/looper.h>
69

710
#include <string_view>
811

912
#include <runtime-base/android-system.hh>
13+
#include <runtime-base/mainthread-dso-loader.hh>
14+
#include <runtime-base/system-loadlibrary-wrapper.hh>
1015
#include <runtime-base/util.hh>
1116
#include <shared/helpers.hh>
1217

@@ -19,13 +24,11 @@ namespace xamarin::android {
1924
{
2025
public:
2126
[[gnu::flatten]]
22-
static void init (JNIEnv *env, jclass systemClass)
27+
static void init (JNIEnv *env, jclass systemClass, ALooper *main_looper, pid_t _main_thread_id) noexcept
2328
{
24-
systemKlass = systemClass;
25-
System_loadLibrary = env->GetStaticMethodID (systemClass, "loadLibrary", "(Ljava/lang/String;)V");
26-
if (System_loadLibrary == nullptr) [[unlikely]] {
27-
Helpers::abort_application ("Failed to look up the Java System.loadLibrary method.");
28-
}
29+
SystemLoadLibraryWrapper::init (env, systemClass);
30+
MainThreadDsoLoader::init (main_looper);
31+
main_thread_id = _main_thread_id;
2932
}
3033

3134
// Overload used to load libraries from the file system.
@@ -68,6 +71,7 @@ namespace xamarin::android {
6871

6972
private:
7073
static auto get_jnienv () noexcept -> JNIEnv*;
74+
static auto load_jni_on_main_thread (std::string_view const& full_name, std::string const& undecorated_name) noexcept -> void*;
7175

7276
[[gnu::always_inline]]
7377
static auto log_and_return (void *handle, std::string_view const& full_name) -> void*
@@ -145,25 +149,29 @@ namespace xamarin::android {
145149
return name;
146150
};
147151

148-
// std::string is needed because we must pass a NUL-terminated string to Java, otherwise
149-
// strange things happen (and std::string_view is not necessarily such a string)
150-
const std::string undecorated_lib_name { get_undecorated_name (name, name_is_path) };
151-
log_debug (LOG_ASSEMBLY, "Undecorated library name: {}", undecorated_lib_name);
152-
153-
JNIEnv *jni_env = get_jnienv ();
154-
jstring lib_name = jni_env->NewStringUTF (undecorated_lib_name.c_str ());
155-
if (lib_name == nullptr) [[unlikely]] {
156-
// It's an OOM, there's nothing better we can do
157-
Helpers::abort_application ("Java string allocation failed while loading a shared library.");
158-
}
159-
jni_env->CallStaticVoidMethod (systemKlass, System_loadLibrary, lib_name);
160-
if (jni_env->ExceptionCheck ()) {
161-
log_debug (LOG_ASSEMBLY, "System.loadLibrary threw a Java exception. Will attempt to log it.");
162-
jni_env->ExceptionDescribe ();
163-
jni_env->ExceptionClear ();
164-
log_debug (LOG_ASSEMBLY, "Java exception cleared");
165-
// TODO: should we abort? Return `nullptr`? `dlopen` still has a chance to succeed, even if loadLibrary
166-
// failed but it won't call `JNI_OnLoad` etc, so the result might be less than perfect.
152+
// So, we have a rather nasty problem here. If we're on a thread other than the main one (or, to be more
153+
// precise - one not created by Java), we will NOT have the special class loader Android uses in JVM and
154+
// which knows about the special application-specific .so paths (like the one inside the APK itself). For
155+
// that reason, `System.loadLibrary` will not be able to find the requested .so and we can't pass it a full
156+
// path to it, since it accepts only the undecorated library name.
157+
// We have to call `System.loadLibrary` on the main thread, so that the special class loader is available to
158+
// it. At the same time, we have to do it synchronously, because we must be able to get the library handle
159+
// **here**. We could call to a Java function here, but then synchronization might be an issue. So, instead,
160+
// we use a wrapper around System.loadLibrary that uses the ALooper native Android interface. It's a bit
161+
// clunky (as it requires using a fake pipe(2) to force the looper to call us on the main thread) but it
162+
// should work across all the Android versions.
163+
164+
// TODO: implement the above
165+
if (gettid () == main_thread_id) {
166+
if (!SystemLoadLibraryWrapper::load (get_jnienv (), get_undecorated_name (name, name_is_path))) {
167+
// We could abort, but let's let the managed land react to this library missing. We cannot continue
168+
// with `dlopen` below, because without `JNI_OnLoad` etc invoked, we might have nasty crashes in the
169+
// library code if e.g. it assumes that `JNI_OnLoad` initialized all the Java class, method etc
170+
// pointers.
171+
return nullptr;
172+
}
173+
} else {
174+
Helpers::abort_application ("Loading DSO on the main thread not implemented yet"sv);
167175
}
168176

169177
// This is unfortunate, but since `System.loadLibrary` doesn't return the class handle, we must get it this
@@ -177,5 +185,6 @@ namespace xamarin::android {
177185
private:
178186
static inline jmethodID System_loadLibrary = nullptr;
179187
static inline jclass systemKlass = nullptr;
188+
static inline pid_t main_thread_id = 0;
180189
};
181190
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
#pragma once
2+
3+
#include <cerrno>
4+
#include <cstring>
5+
#include <unistd.h>
6+
7+
#include <format>
8+
#include <semaphore>
9+
#include <string_view>
10+
11+
#include <android/looper.h>
12+
13+
#include <shared/helpers.hh>
14+
15+
namespace xamarin::android {
16+
// This class is **strictly** one-shot-per-instance! That is, the `load` method mustn't be called on the
17+
// same object more than once. This is by design, to make the code simpler.
18+
class MainThreadDsoLoader
19+
{
20+
public:
21+
explicit MainThreadDsoLoader () noexcept
22+
{
23+
if (pipe (pipe_fds) != 0) {
24+
Helpers::abort_application (
25+
LOG_ASSEMBLY,
26+
std::format (
27+
"Failed to create a pipe for main thread DSO loader. {}",
28+
strerror (errno)
29+
)
30+
);
31+
}
32+
33+
int ret = ALooper_addFd (
34+
main_thread_looper,
35+
pipe_fds[0],
36+
ALOOPER_POLL_CALLBACK,
37+
ALOOPER_EVENT_INPUT,
38+
load_cb,
39+
this
40+
);
41+
42+
if (ret == -1) {
43+
Helpers::abort_application ("Failed to init main looper with pipe file descriptors in the main thread DSO loader"sv);
44+
}
45+
}
46+
47+
MainThreadDsoLoader (const MainThreadDsoLoader&) = delete;
48+
MainThreadDsoLoader (MainThreadDsoLoader&&) = delete;
49+
50+
virtual ~MainThreadDsoLoader () noexcept
51+
{
52+
if (pipe_fds[0] != -1) {
53+
ALooper_removeFd (main_thread_looper, pipe_fds[0]);
54+
close (pipe_fds[0]);
55+
}
56+
57+
if (pipe_fds[1] != -1) {
58+
close (pipe_fds[1]);
59+
}
60+
61+
// No need to release the looper, it needs to stay acquired.
62+
}
63+
64+
MainThreadDsoLoader& operator=(const MainThreadDsoLoader&) = delete;
65+
MainThreadDsoLoader& operator=(MainThreadDsoLoader&&) = delete;
66+
67+
void load (std::string_view const& full_name, std::string const& undecorated_name) noexcept
68+
{
69+
// TODO: init load here by writing to pipe_fds[1]
70+
71+
// Wait for the callback to complete
72+
load_complete_sem.acquire ();
73+
}
74+
75+
static void init (ALooper *main_looper)
76+
{
77+
main_thread_looper = main_looper;
78+
79+
// This will keep the looper around for the lifetime of the application.
80+
ALooper_acquire (main_looper);
81+
}
82+
83+
private:
84+
static auto load_cb (int fd, int events, void *data) noexcept -> int
85+
{
86+
auto self = reinterpret_cast<MainThreadDsoLoader*> (data);
87+
88+
self->load_complete_sem.release ();
89+
return 0; // We're one-shot, 0 means just that
90+
}
91+
92+
private:
93+
int pipe_fds[2] = {-1, -1};
94+
std::binary_semaphore load_complete_sem {0};
95+
96+
static inline ALooper *main_thread_looper = nullptr;
97+
};
98+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
#pragma once
2+
3+
#include <string>
4+
#include <string_view>
5+
6+
#include <jni.h>
7+
8+
#include <shared/helpers.hh>
9+
#include <runtime-base/logger.hh>
10+
11+
namespace xamarin::android {
12+
class SystemLoadLibraryWrapper
13+
{
14+
public:
15+
[[gnu::flatten]]
16+
static void init (JNIEnv *env, jclass systemClass) noexcept
17+
{
18+
systemKlass = systemClass;
19+
System_loadLibrary = env->GetStaticMethodID (systemClass, "loadLibrary", "(Ljava/lang/String;)V");
20+
if (System_loadLibrary == nullptr) [[unlikely]] {
21+
Helpers::abort_application ("Failed to look up the Java System.loadLibrary method.");
22+
}
23+
}
24+
25+
[[gnu::flatten]]
26+
static auto load (JNIEnv *jni_env, std::string_view const& undecorated_lib_name) noexcept -> bool
27+
{
28+
// std::string is needed because we must pass a NUL-terminated string to Java, otherwise
29+
// strange things happen (and std::string_view is not necessarily such a string)
30+
const std::string lib_name { undecorated_lib_name };
31+
log_debug (LOG_ASSEMBLY, "Undecorated library name: {}", lib_name);
32+
33+
jstring java_lib_name = jni_env->NewStringUTF (lib_name.c_str ());
34+
if (java_lib_name == nullptr) [[unlikely]] {
35+
// It's an OOM, there's nothing better we can do
36+
Helpers::abort_application ("Java string allocation failed while loading a shared library.");
37+
}
38+
jni_env->CallStaticVoidMethod (systemKlass, System_loadLibrary, java_lib_name);
39+
if (jni_env->ExceptionCheck ()) {
40+
log_debug (LOG_ASSEMBLY, "System.loadLibrary threw a Java exception. Will attempt to log it.");
41+
jni_env->ExceptionDescribe ();
42+
jni_env->ExceptionClear ();
43+
log_debug (LOG_ASSEMBLY, "Java exception cleared");
44+
return false;
45+
}
46+
47+
return true;
48+
}
49+
50+
private:
51+
static inline jmethodID System_loadLibrary = nullptr;
52+
static inline jclass systemKlass = nullptr;
53+
};
54+
}

src/native/common/runtime-base/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ set(LIB_NAME runtime-base-common)
22
set(LIB_ALIAS xa::runtime-base-common)
33

44
set(XA_RUNTIME_BASE_SOURCES
5+
dso-loader.cc
56
timing-internal.cc
67
)
78
add_clang_check_sources("${XA_RUNTIME_BASE_SOURCES}")
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#include <runtime-base/dso-loader.hh>
2+
#include <runtime-base/mainthread-dso-loader.hh>
3+
4+
using namespace xamarin::android;
5+
6+
auto DsoLoader::load_jni_on_main_thread (std::string_view const& full_name, std::string const& undecorated_name) noexcept -> void*
7+
{
8+
return nullptr;
9+
}

src/native/mono/monodroid/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ macro(lib_target_options TARGET_NAME)
233233
xa::pinvoke-override-precompiled
234234
xa::lz4
235235
-lmonosgen-2.0
236+
-landroid
236237
-llog
237238
)
238239
endmacro ()

src/native/mono/monodroid/monodroid-glue.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1390,7 +1390,12 @@ MonodroidRuntime::Java_mono_android_Runtime_initInternal (JNIEnv *env, jclass kl
13901390
AndroidSystem::set_running_in_emulator (isEmulator);
13911391

13921392
java_System = RuntimeUtil::get_class_from_runtime_field (env, klass, "java_lang_System", true);
1393-
DsoLoader::init (env, java_System);
1393+
DsoLoader::init (
1394+
env,
1395+
RuntimeUtil::get_class_from_runtime_field (env, klass, "java_lang_System", true),
1396+
ALooper_forThread (), // main thread looper
1397+
gettid ()
1398+
);
13941399

13951400
java_TimeZone = RuntimeUtil::get_class_from_runtime_field (env, klass, "java_util_TimeZone", true);
13961401

0 commit comments

Comments
 (0)