Skip to content

Commit

Permalink
do not attempt to acquire a write lock when resolving so dependencies
Browse files Browse the repository at this point in the history
Summary: ReentrantReadWriteLocks do not allow promotion of read locks to write lock, causing a deadlock when this is attempted.  This changes the logic to only run the source library reparsing when we're at the root level and do not already have the lock.

Reviewed By: JamieEi, hick209

Differential Revision: D13581081

fbshipit-source-id: 33432700ac0913365127e540c746ec95a1a7710e
  • Loading branch information
joelmccall authored and facebook-github-bot committed Jan 4, 2019
1 parent ebc9e1e commit ea661f5
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
4 changes: 3 additions & 1 deletion java/com/facebook/soloader/DirectorySoSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ private void loadDependencies(File soFile, int loadFlags, StrictMode.ThreadPolic
}

SoLoader.loadLibraryBySoName(
dependency, (loadFlags | LOAD_FLAG_ALLOW_IMPLICIT_PROVISION), threadPolicy);
dependency,
(loadFlags | LOAD_FLAG_ALLOW_IMPLICIT_PROVISION & ~LOAD_FLAG_ALLOW_SOURCE_CHANGE),
threadPolicy);
}
}

Expand Down
10 changes: 8 additions & 2 deletions java/com/facebook/soloader/SoLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,11 @@ public static boolean loadLibrary(String shortName, int loadFlags) throws Unsati
String soName = mergedLibName != null ? mergedLibName : shortName;

return loadLibraryBySoName(
System.mapLibraryName(soName), shortName, mergedLibName, loadFlags, null);
System.mapLibraryName(soName),
shortName,
mergedLibName,
loadFlags | SoSource.LOAD_FLAG_ALLOW_SOURCE_CHANGE,
null);
}

/* package */ static void loadLibraryBySoName(
Expand Down Expand Up @@ -658,7 +662,9 @@ private static void doLoadLibraryBySoName(
} finally {
sSoSourcesLock.readLock().unlock();
}
if (result == SoSource.LOAD_RESULT_NOT_FOUND) {
if ((loadFlags & SoSource.LOAD_FLAG_ALLOW_SOURCE_CHANGE)
== SoSource.LOAD_FLAG_ALLOW_SOURCE_CHANGE
&& result == SoSource.LOAD_RESULT_NOT_FOUND) {
sSoSourcesLock.writeLock().lock();
try {
// TODO(T26270128): check and update sBackupSoSource as well
Expand Down
9 changes: 6 additions & 3 deletions java/com/facebook/soloader/SoSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,14 @@ abstract public class SoSource {
/** Allow loadLibrary to implicitly provide the library instead of actually loading it. */
public static final int LOAD_FLAG_ALLOW_IMPLICIT_PROVISION = 1;

/** Allow loadLibrary to reparse the so sources directories. */
public static final int LOAD_FLAG_ALLOW_SOURCE_CHANGE = 1 << 1;

/**
* Min flag that can be used in customized {@link SoFileLoader#load(String, int)}
* implementation. The custom flag value has to be greater than this.
* Min flag that can be used in customized {@link SoFileLoader#load(String, int)} implementation.
* The custom flag value has to be greater than this.
*/
public static final int LOAD_FLAG_MIN_CUSTOM_FLAG = 1 << 1;
public static final int LOAD_FLAG_MIN_CUSTOM_FLAG = LOAD_FLAG_ALLOW_SOURCE_CHANGE << 1;

/**
* Allow prepare to spawn threads to do background work.
Expand Down

0 comments on commit ea661f5

Please sign in to comment.