Skip to content

Commit

Permalink
make AsyncStorage serially execute requests (facebook#18522)
Browse files Browse the repository at this point in the history
Summary:
This patch is a bit of a hack job, but I'd argue it's necessary to dramatically improve the dev UX on Android devices. Somewhere in react-native, there's a shared SerialExecutor which AsyncStorage uses that is getting blocked, causing remote debugging to occasionally hang indefinitely for folks making AsyncStorage requests. This is frustrating from a dev UX perspective, and has persisted across several versions as far back as RN 0.44, and still remains on RN 0.54.

The issue seems to only happen on Android > 7+, which is likely because the ThreadPoolExecutor behavior changed in this version:
https://stackoverflow.com/questions/9654148/android-asynctask-threads-limits

Fixes facebook#14101

We've been using this patch in production for the past 4 months on our team by overriding the AsyncStorage native module. We use AsyncStorage extensively for offline data and caching.

You can test by compiling this commit version into a test react native repository that is set to build from source:

```sh
git clone https://github.com/dannycochran/react-native rnAsyncStorage
cd rnAsyncStorage
git checkout asyncStorage
cd ..
git clone https://github.com/dannycochran/asyncStorageTest
yarn install
cp -r ../rnAsyncStorage node_modules/react-native
react-native run-android
```

No documentation change is required.

facebook#16905

[Android] [BUGFIX] [AsyncStorage] - Fix AsyncStorage causing remote debugger to hang indefinitely.
Closes facebook#18522

Differential Revision: D8624088

fbshipit-source-id: 2a599fbca270a4826063cd93be8eaf796f34f57a
  • Loading branch information
Daniel Cochran authored and facebook-github-bot committed Jun 28, 2018
1 parent 3cd0737 commit 953f91a
Showing 1 changed file with 36 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@

package com.facebook.react.modules.storage;

import java.util.ArrayDeque;
import java.util.HashSet;
import java.util.concurrent.Executor;

import android.database.Cursor;
import android.database.sqlite.SQLiteStatement;
import android.os.AsyncTask;

import com.facebook.common.logging.FLog;
import com.facebook.react.bridge.Arguments;
Expand Down Expand Up @@ -43,6 +46,33 @@ public final class AsyncStorageModule
private ReactDatabaseSupplier mReactDatabaseSupplier;
private boolean mShuttingDown = false;

// Borrowed from https://android.googlesource.com/platform/frameworks/base.git/+/1488a3a19d4681a41fb45570c15e14d99db1cb66/core/java/android/os/AsyncTask.java#237
private static class SerialExecutor implements Executor {
final ArrayDeque<Runnable> mTasks = new ArrayDeque<Runnable>();
Runnable mActive;
public synchronized void execute(final Runnable r) {
mTasks.offer(new Runnable() {
public void run() {
try {
r.run();
} finally {
scheduleNext();
}
}
});
if (mActive == null) {
scheduleNext();
}
}
protected synchronized void scheduleNext() {
if ((mActive = mTasks.poll()) != null) {
AsyncTask.THREAD_POOL_EXECUTOR.execute(mActive);
}
}
}

private static final Executor ASYNC_STORAGE_EXECUTOR = new SerialExecutor();

public AsyncStorageModule(ReactApplicationContext reactContext) {
super(reactContext);
mReactDatabaseSupplier = ReactDatabaseSupplier.getInstance(reactContext);
Expand Down Expand Up @@ -141,7 +171,7 @@ protected void doInBackgroundGuarded(Void... params) {

callback.invoke(null, data);
}
}.execute();
}.executeOnExecutor(ASYNC_STORAGE_EXECUTOR);
}

/**
Expand Down Expand Up @@ -208,7 +238,7 @@ protected void doInBackgroundGuarded(Void... params) {
callback.invoke();
}
}
}.execute();
}.executeOnExecutor(ASYNC_STORAGE_EXECUTOR);
}

/**
Expand Down Expand Up @@ -259,7 +289,7 @@ protected void doInBackgroundGuarded(Void... params) {
callback.invoke();
}
}
}.execute();
}.executeOnExecutor(ASYNC_STORAGE_EXECUTOR);
}

/**
Expand Down Expand Up @@ -322,7 +352,7 @@ protected void doInBackgroundGuarded(Void... params) {
callback.invoke();
}
}
}.execute();
}.executeOnExecutor(ASYNC_STORAGE_EXECUTOR);
}

/**
Expand All @@ -345,7 +375,7 @@ protected void doInBackgroundGuarded(Void... params) {
callback.invoke(AsyncStorageErrorUtil.getError(null, e.getMessage()));
}
}
}.execute();
}.executeOnExecutor(ASYNC_STORAGE_EXECUTOR);
}

/**
Expand Down Expand Up @@ -379,7 +409,7 @@ protected void doInBackgroundGuarded(Void... params) {
}
callback.invoke(null, data);
}
}.execute();
}.executeOnExecutor(ASYNC_STORAGE_EXECUTOR);
}

/**
Expand Down

0 comments on commit 953f91a

Please sign in to comment.