Skip to content

Commit

Permalink
Release lib lock before JniOnLoad to avoid deadlock on re-entry
Browse files Browse the repository at this point in the history
Summary: Re-entering SoLoader.loadLibrary within invokeJniOnload can lead to deadlock due to the hold of loadingLibLock. We trust invokeJniOnload to handle this correctly without the lock.

Reviewed By: simpleton

Differential Revision: D23720143

fbshipit-source-id: 07d3d3e327b5d7bc26a2d4dd0557524e61c0010f
  • Loading branch information
Adam Iser authored and facebook-github-bot committed Sep 21, 2020
1 parent cd51d00 commit 06e415e
Showing 1 changed file with 39 additions and 39 deletions.
78 changes: 39 additions & 39 deletions java/com/facebook/soloader/SoLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -737,47 +737,47 @@ private static boolean loadLibraryBySoNameImpl(
}
}
}
}

if ((loadFlags & SOLOADER_SKIP_MERGED_JNI_ONLOAD) == 0) {
boolean isAlreadyMerged =
!TextUtils.isEmpty(shortName) && sLoadedAndMergedLibraries.contains(shortName);
if (mergedLibName != null && !isAlreadyMerged) {
if ((loadFlags & SOLOADER_SKIP_MERGED_JNI_ONLOAD) == 0) {
boolean isAlreadyMerged =
!TextUtils.isEmpty(shortName) && sLoadedAndMergedLibraries.contains(shortName);
if (mergedLibName != null && !isAlreadyMerged) {
if (SYSTRACE_LIBRARY_LOADING) {
Api18TraceUtils.beginTraceSection("MergedSoMapping.invokeJniOnload[", shortName, "]");
}
try {
Log.d(TAG, "About to merge: " + shortName + " / " + soName);
MergedSoMapping.invokeJniOnload(shortName);
sLoadedAndMergedLibraries.add(shortName);
} catch (UnsatisfiedLinkError e) {
// If you are seeing this exception, first make sure your library sets
// allow_jni_merging=True. Trying to merge a library without that
// will trigger this error. If that's already in place, you're probably
// not defining JNI_OnLoad. Calling SoLoader.loadLibrary on a library
// that doesn't define JNI_OnLoad is a no-op when that library is not merged.
// Once you enable merging, it throws an UnsatisfiedLinkError.
// There are three main reasons a library might not define JNI_OnLoad,
// and the solution depends on which case you have.
// - You might be using implicit registration (native methods defined like
// `Java_com_facebook_Foo_bar(JNIEnv* env)`). This is not safe on Android
// https://fb.workplace.com/groups/442333009148653/permalink/651212928260659/
// and is not compatible with FBJNI. Stop doing it. Use FBJNI registerNatives.
// - You might have a C++-only library with no JNI bindings and no static
// initializers with side-effects. You can just delete the loadLibrary call.
// - You might have a C++-only library that needs to be loaded explicitly because
// it has static initializers whose side-effects are needed. In that case,
// pass the SOLOADER_SKIP_MERGED_JNI_ONLOAD flag to loadLibrary.
throw new RuntimeException(
"Failed to call JNI_OnLoad from '"
+ shortName
+ "', which has been merged into '"
+ soName
+ "'. See comment for details.",
e);
} finally {
if (SYSTRACE_LIBRARY_LOADING) {
Api18TraceUtils.beginTraceSection("MergedSoMapping.invokeJniOnload[", shortName, "]");
}
try {
Log.d(TAG, "About to merge: " + shortName + " / " + soName);
MergedSoMapping.invokeJniOnload(shortName);
sLoadedAndMergedLibraries.add(shortName);
} catch (UnsatisfiedLinkError e) {
// If you are seeing this exception, first make sure your library sets
// allow_jni_merging=True. Trying to merge a library without that
// will trigger this error. If that's already in place, you're probably
// not defining JNI_OnLoad. Calling SoLoader.loadLibrary on a library
// that doesn't define JNI_OnLoad is a no-op when that library is not merged.
// Once you enable merging, it throws an UnsatisfiedLinkError.
// There are three main reasons a library might not define JNI_OnLoad,
// and the solution depends on which case you have.
// - You might be using implicit registration (native methods defined like
// `Java_com_facebook_Foo_bar(JNIEnv* env)`). This is not safe on Android
// https://fb.workplace.com/groups/442333009148653/permalink/651212928260659/
// and is not compatible with FBJNI. Stop doing it. Use FBJNI registerNatives.
// - You might have a C++-only library with no JNI bindings and no static
// initializers with side-effects. You can just delete the loadLibrary call.
// - You might have a C++-only library that needs to be loaded explicitly because
// it has static initializers whose side-effects are needed. In that case,
// pass the SOLOADER_SKIP_MERGED_JNI_ONLOAD flag to loadLibrary.
throw new RuntimeException(
"Failed to call JNI_OnLoad from '"
+ shortName
+ "', which has been merged into '"
+ soName
+ "'. See comment for details.",
e);
} finally {
if (SYSTRACE_LIBRARY_LOADING) {
Api18TraceUtils.endSection();
}
Api18TraceUtils.endSection();
}
}
}
Expand Down

0 comments on commit 06e415e

Please sign in to comment.