OverlappingFileLockException in 0.10.3

[jbettcher]

I'm using SoLoader in an app that makes multi-threaded SoLoader.init() and SoLoader.loadLibrary() calls for several different native SO libraries during application startup. The application recently switched from 0.10.1 to 0.10.3, and it looks like the crash below happens somewhat randomly (likely a race condition) after upgrading.

If I had to guess, it looks like anything that calls SoSource#prepare needs to hold a sSoSourcesLock.writeLock(). However, loadLibraryBySoNameImpl seems to only hold a read lock while calling backupSoSource.prepare. I'm not an expert on this code, but I think multiple threads can acquire a read lock at the same time.

java.nio.channels.OverlappingFileLockException
        at android.app.ActivityThread.handleBindApplication(ActivityThread.java:6730)
        at android.app.ActivityThread.access$1500(ActivityThread.java:247)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2053)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7839)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
     Caused by: java.nio.channels.OverlappingFileLockException
        at sun.nio.ch.SharedFileLockTable.checkList(FileLockTable.java:255)
        at sun.nio.ch.SharedFileLockTable.add(FileLockTable.java:152)
        at sun.nio.ch.FileChannelImpl.tryLock(FileChannelImpl.java:1174)
        at java.nio.channels.FileChannel.tryLock(FileChannel.java:1155)
        at com.facebook.soloader.FileLocker.init(FileLocker.java:50)
        at com.facebook.soloader.FileLocker.<init>(FileLocker.java:68)
        at com.facebook.soloader.FileLocker.tryLock(FileLocker.java:36)
        at com.facebook.soloader.SysUtil.getOrCreateLockOnDir(SysUtil.java:413)
        at com.facebook.soloader.UnpackingSoSource.getOrCreateLock(UnpackingSoSource.java:492)
        at com.facebook.soloader.UnpackingSoSource.prepare(UnpackingSoSource.java:511)
        at com.facebook.soloader.SoLoader.initSoSources(SoLoader.java:306)
        at com.facebook.soloader.SoLoader.init(SoLoader.java:239)
        at com.facebook.soloader.SoLoader.init(SoLoader.java:255)

[jbettcher]

Just FYI reverting to 0.10.1 has fixed the issue for now. Also, attempting to change the readLock() to a writeLock() just results in a deadlock during initialization, so I don't think this is the right fix.

[mhrheaume]

I've been talking to @jbettcher on the side about the issue, but just to fill everyone else in, we now think the issue is actually in SoLoader.init rather than in SoLoader.loadLibrary. Consider the following scenario:

1. Thread A calls SoLoader.init and enters initSoSources. Assume this is the first call to initSoSources, so sSoSources is null and the thread grabs the write lock (sSoSourcesLock.writeLock().lock()).

2. Thread A with the write lock starts to go through the process of initializing sSoSources.

3. Thread B calls SoLoader.init and enters initSoSources. Since thread A is still initializing sSoSources, it's still null, so thread B passes the null check and waits for the write lock.

4. Thread A finishes initializing sSoSources and releases the write lock.

5. Thread B grabs the write lock and starts to re-initialize sSoSources, which results in the OverlappingFileLockException.

This isn't an issue on 0.10.1 (the version we are currently on) because in initSoSources in 0.10.1, the null check is performed inside the write lock.

https://github.com/facebook/SoLoader/blob/v0.10.1/java/com/facebook/soloader/SoLoader.java#L225

Checking for nullability to early exit prior to grabbing the write lock is a perfectly reasonable optimization here, I think we just need to double check that sSoSources is in fact still null once the write lock has been obtained to prevent the scenario described above from occurring.

I'll have a separate PR up shortly to address the issue, I'm just trying to test a bit more before putting it up (getting Buck set up on my M1 was.. an interesting adventure to say the least 😆).

/cc @passy since you mentioned you were looking to route @jbettcher's original PR.