status-im
/
react-native
mirror of https://github.com/status-im/react-native.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

make AsyncStorage serially execute requests (#18522) 

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 #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.

https://github.com/facebook/react-native/pull/16905

[Android] [BUGFIX] [AsyncStorage] - Fix AsyncStorage causing remote debugger to hang indefinitely.
Pull Request resolved: https://github.com/facebook/react-native/pull/18522

Differential Revision: D8624088

Pulled By: hramos

fbshipit-source-id: a1d2e3458d98467845cb34ac73f2aafaaa15ace2
This commit is contained in:
Daniel Cochran 2018-07-30 12:00:35 -07:00 committed by Facebook Github Bot
parent 82af7c989b
commit 1b09bd7fba
2 changed files with 82 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java
View File

@ -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;
@ -23,6 +26,7 @@ import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.bridge.WritableArray;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.common.annotations.VisibleForTesting;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.modules.common.ModuleDataCleaner;
@ -43,8 +47,47 @@ public final class AsyncStorageModule
private ReactDatabaseSupplier mReactDatabaseSupplier;
private boolean mShuttingDown = false;
// Adapted from https://android.googlesource.com/platform/frameworks/base.git/+/1488a3a19d4681a41fb45570c15e14d99db1cb66/core/java/android/os/AsyncTask.java#237
private class SerialExecutor implements Executor {
private final ArrayDeque<Runnable> mTasks = new ArrayDeque<Runnable>();
private Runnable mActive;
private final Executor executor;
SerialExecutor(Executor executor) {
this.executor = executor;
}
public synchronized void execute(final Runnable r) {
mTasks.offer(new Runnable() {
public void run() {
try {
r.run();
} finally {
scheduleNext();
}
}
});
if (mActive == null) {
scheduleNext();
}
}
synchronized void scheduleNext() {
if ((mActive = mTasks.poll()) != null) {
executor.execute(mActive);
}
}
}
private final SerialExecutor executor;
public AsyncStorageModule(ReactApplicationContext reactContext) {
this(reactContext, AsyncTask.THREAD_POOL_EXECUTOR);
}
@VisibleForTesting
AsyncStorageModule(ReactApplicationContext reactContext, Executor executor) {
super(reactContext);
this.executor = new SerialExecutor(executor);
mReactDatabaseSupplier = ReactDatabaseSupplier.getInstance(reactContext);
}
@ -141,7 +184,7 @@ public final class AsyncStorageModule
callback.invoke(null, data);
}
}.execute();
}.executeOnExecutor(executor);
}
/**
@ -208,7 +251,7 @@ public final class AsyncStorageModule
callback.invoke();
}
}
}.execute();
}.executeOnExecutor(executor);
}
/**
@ -259,7 +302,7 @@ public final class AsyncStorageModule
callback.invoke();
}
}
}.execute();
}.executeOnExecutor(executor);
}
/**
@ -322,7 +365,7 @@ public final class AsyncStorageModule
callback.invoke();
}
}
}.execute();
}.executeOnExecutor(executor);
}
/**
@ -345,7 +388,7 @@ public final class AsyncStorageModule
callback.invoke(AsyncStorageErrorUtil.getError(null, e.getMessage()));
}
}
}.execute();
}.executeOnExecutor(executor);
}
/**
@ -379,7 +422,7 @@ public final class AsyncStorageModule
}
callback.invoke(null, data);
}
}.execute();
}.executeOnExecutor(executor);
}
/**

49
ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java
View File

@ -10,13 +10,16 @@ package com.facebook.react.modules.storage;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.Executor;
import android.app.Activity;
import android.content.Context;
import android.content.ContextWrapper;
import android.os.AsyncTask;
import com.facebook.react.bridge.Arguments;
import com.facebook.react.bridge.Callback;
import com.facebook.react.bridge.GuardedAsyncTask;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactTestHelper;
import com.facebook.react.bridge.JavaOnlyArray;
@ -35,6 +38,7 @@ import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.mockito.verification.VerificationMode;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
@ -42,7 +46,7 @@ import org.powermock.modules.junit4.rule.PowerMockRule;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.Robolectric;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;
import org.robolectric.util.concurrent.RoboExecutorService;
import static org.mockito.Mockito.mock;
import static org.fest.assertions.api.Assertions.assertThat;
@ -81,7 +85,10 @@ public class AsyncStorageModuleTest {
});
// don't use Robolectric before initializing mocks
mStorage = new AsyncStorageModule(ReactTestHelper.createCatalystContextForTest());
mStorage = new AsyncStorageModule(
ReactTestHelper.createCatalystContextForTest(),
new RoboExecutorService()
);
mEmptyArray = new JavaOnlyArray();
}
@ -104,7 +111,7 @@ public class AsyncStorageModuleTest {
Callback setCallback = mock(Callback.class);
mStorage.multiSet(keyValues, setCallback);
Mockito.verify(setCallback, Mockito.times(1)).invoke();
verify(setCallback, Mockito.times(1)).invoke();
JavaOnlyArray keys = new JavaOnlyArray();
keys.pushString(key1);
@ -112,7 +119,7 @@ public class AsyncStorageModuleTest {
Callback getCallback = mock(Callback.class);
mStorage.multiGet(keys, getCallback);
Mockito.verify(getCallback, Mockito.times(1)).invoke(null, keyValues);
verify(getCallback, Mockito.times(1)).invoke(null, keyValues);
keys.pushString(fakeKey);
JavaOnlyArray row3 = new JavaOnlyArray();
@ -122,7 +129,7 @@ public class AsyncStorageModuleTest {
Callback getCallback2 = mock(Callback.class);
mStorage.multiGet(keys, getCallback2);
Mockito.verify(getCallback2, Mockito.times(1)).invoke(null, keyValues);
verify(getCallback2, Mockito.times(1)).invoke(null, keyValues);
}
@Test
@ -143,22 +150,22 @@ public class AsyncStorageModuleTest {
Callback getCallback = mock(Callback.class);
mStorage.multiRemove(keys, getCallback);
Mockito.verify(getCallback, Mockito.times(1)).invoke();
verify(getCallback, Mockito.times(1)).invoke();
Callback getAllCallback = mock(Callback.class);
mStorage.getAllKeys(getAllCallback);
Mockito.verify(getAllCallback, Mockito.times(1)).invoke(null, mEmptyArray);
verify(getAllCallback, Mockito.times(1)).invoke(null, mEmptyArray);
mStorage.multiSet(keyValues, mock(Callback.class));
keys.pushString("fakeKey");
Callback getCallback2 = mock(Callback.class);
mStorage.multiRemove(keys, getCallback2);
Mockito.verify(getCallback2, Mockito.times(1)).invoke();
verify(getCallback2, Mockito.times(1)).invoke();
Callback getAllCallback2 = mock(Callback.class);
mStorage.getAllKeys(getAllCallback2);
Mockito.verify(getAllCallback2, Mockito.times(1)).invoke(null, mEmptyArray);
verify(getAllCallback2, Mockito.times(1)).invoke(null, mEmptyArray);
}
@Test
@ -175,7 +182,7 @@ public class AsyncStorageModuleTest {
{
Callback callback = mock(Callback.class);
mStorage.multiGet(getArray(mergeKey), callback);
Mockito.verify(callback, Mockito.times(1))
verify(callback, Mockito.times(1))
.invoke(null, JavaOnlyArray.of(getArray(mergeKey, value.toString())));
}
@ -200,7 +207,7 @@ public class AsyncStorageModuleTest {
value.put("foo2", createJSONObject("key1", "val3", "key2", "val2"));
Callback callback = mock(Callback.class);
mStorage.multiGet(getArray(mergeKey), callback);
Mockito.verify(callback, Mockito.times(1))
verify(callback, Mockito.times(1))
.invoke(null, JavaOnlyArray.of(getArray(mergeKey, value.toString())));
}
@ -219,18 +226,18 @@ public class AsyncStorageModuleTest {
Callback getAllCallback = mock(Callback.class);
mStorage.getAllKeys(getAllCallback);
Mockito.verify(getAllCallback, Mockito.times(1)).invoke(null, storedKeys);
verify(getAllCallback, Mockito.times(1)).invoke(null, storedKeys);
Callback getAllCallback2 = mock(Callback.class);
mStorage.multiRemove(getArray(keys[0]), mock(Callback.class));
mStorage.getAllKeys(getAllCallback2);
Mockito.verify(getAllCallback2, Mockito.times(1)).invoke(null, getArray(keys[1]));
verify(getAllCallback2, Mockito.times(1)).invoke(null, getArray(keys[1]));
mStorage.multiRemove(getArray(keys[1]), mock(Callback.class));
Callback getAllCallback3 = mock(Callback.class);
mStorage.getAllKeys(getAllCallback3);
Mockito.verify(getAllCallback3, Mockito.times(1)).invoke(null, mEmptyArray);
verify(getAllCallback3, Mockito.times(1)).invoke(null, mEmptyArray);
}
@Test
@ -242,11 +249,11 @@ public class AsyncStorageModuleTest {
Callback clearCallback2 = mock(Callback.class);
mStorage.clear(clearCallback2);
Mockito.verify(clearCallback2, Mockito.times(1)).invoke();
verify(clearCallback2, Mockito.times(1)).invoke();
Callback getAllCallback2 = mock(Callback.class);
mStorage.getAllKeys(getAllCallback2);
Mockito.verify(getAllCallback2, Mockito.times(1)).invoke(null, mEmptyArray);
verify(getAllCallback2, Mockito.times(1)).invoke(null, mEmptyArray);
}
@Test
@ -339,4 +346,14 @@ public class AsyncStorageModuleTest {
}
return array;
}
private static void waitForAsync() {
Robolectric.flushForegroundThreadScheduler();
Robolectric.flushBackgroundThreadScheduler();
}
private static <T> T verify(T mock, VerificationMode mode) {
waitForAsync();
return Mockito.verify(mock, mode);
}
}