Fix AsyncStorage not resolving its promises

[jsdario]

Motivation

Projects with other background activities may seen its AsyncStorage problems delayed or never resolved. Here it is an old issue explaining the bug conditions for devices with Android 7+

#14101 (comment)

Test Plan

We can reproduce the error following the steps in issues, which I copy paste here:

The following code executes inside React Native component lifecycle's

  componentDidMount() {
    setInterval(async () => {
      console.log('Inside setInterval')
      const data = await AsyncStorage.getAllKeys()
      console.log('inside the getAllKeys')
      data.forEach(async k => {
        const value = await AsyncStorage.getItem(k)
        console.group(k)
        console.log(value)
        console.groupEnd()
      })
    }, 3000)
  }

Code after 'Inside setInterval' is never called. It only runs once if outside the setInterval. If I call once the code outside the setInterval it appears to run just fine. I also tried callback format vs async / await version but it does not seem to matter.

Release Notes

[ANDROID] [BUGFIX] [ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java] - Fixed AsyncStorage when not resolving its promises for Android 7+

[facebook-github-bot]

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

[facebook-github-bot]

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[jsdario]

The same thing happened with other PRs of mine, that have been accepted as well. If I can do something to fix / improve this, let me know!

[mbret]

Hey @jsdario is there any chance for you to update your PR ? Do you need help or anything ?

[jsdario]

I believe it's ready to merge, I am using this fork on production, no surprises so far > 5000 devices

[dannycochran]

Is it possible to do a release, 0.51.1, with this in there? AsyncStorage being broken likely is blocking a lot of apps, like mine, that assume the promise will always resolve.

[esprehn]

How does this compile? Did you mean executeOnExecutor? This seems to pass AsyncTask.THREAD_POOL_EXECUTOR into a bunch of stuff that doesn't actually accept an executor (ex SQLiteStatement or AsyncTask#execute).

[RickDT]

@esprehn that is why the CircleCI tests are failing. It should be executeOnExecutor. I'm also concerned about arbitrarily changing AsyncStorage from being serial, which guarantees order, to parallel, which would have no such guarantee. For instance, if I use AsyncStorage to save a new value for a key 3 times in a row and then immediately read it back out, there's no guarantee I will get the third value. That would probably cause a lot of knock-on effects for developers.

However, AsyncStorage does have potential deadlocks with other uses of AsyncTasks in the app. In my app, we were able to change to parallel tasks to avoid the deadlock.

Ideally, AsyncStorage's use of AsyncTasks would place the work on it's own serial Executor instead of the default one. I'm not familiar enough with Java to competently implement this or I'd jump in :/ (I just wrote my first copy-pasta Java code about a month ago, when I ran across this issue)

[esprehn]

@RickDT I did that by copying the SerialExecutor [1] into a forked version of AsyncStorageModule (and friends).

It's alarming that something is blocking the shared SERIAL_EXECUTOR though. It doesn't seem like it's been identified in the bug, does anyone know what it is?

[1] https://android.googlesource.com/platform/frameworks/base.git/+/1488a3a19d4681a41fb45570c15e14d99db1cb66/core/java/android/os/AsyncTask.java#237

[RickDT]

I believe this happens due to other Java code in the app, outside of React Native, behaving badly and blocking the shared SERIAL_EXECUTOR.

The OP claims "Projects with other background activities...", which is certainly where we saw it in our codebase.

The test case in the OP may be failing due to a bug in the test case. I created a new RN app and pasted the test code and indeed did not get the inner log. However, that's because nothing had been saved in AsyncStorage. The updated test case does work as expected with AsyncStorage:

async componentDidMount() {
  const intervalMillis = 3000;

  //save something first
  await AsyncStorage.setItem("test1", "this is a good test");  

  setInterval(async () => {
    console.log("Inside setInterval");
    const data = await AsyncStorage.getAllKeys();
    console.log("inside the getAllKeys");
    data.forEach(async k => {
      const value = await AsyncStorage.getItem(k);
      console.group(k);
      console.log(value);
      console.groupEnd();
    });
  }, intervalMillis);
}

This will run successfully over many iterations, and if you set intervalMillis to 0 in order to really test the theory, it also runs continuously without blocking.

I think that putting AsyncStorage on a non-shared serial executor will solve the problem for most cases. Other cases may be another bug altogether. If not, we need a true failing test case. If needed, I can whip up a test project that demonstrates blocking from outside of RN in a misbehaving AsyncTask.

[julestruong]

i believe it is fixed in v51

[sunnylqm]

What is the status of this PR now? @shergin

[dannycochran]

With your code snippet, @RickDT, and remote debugging running, if I refresh the default react-native app example app fast enough and often enough, eventually the first call to AsyncStorage will hang indefinitely and never resolve. If I turn off remote debugging, the call will never hang.

Are others experiencing this problem with remote debugging turned off?

[facebook-github-bot]

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[dannycochran]

As far as I can tell, the change in this commit does not actually work.

We've been using the following change in production for the past 4 months and it works:
#18522

[RickDT]

The solution from @dannycochran has two advantages: staying with Sync execution and not relying on shared system executors. I would much prefer it get merged.

I think the root of the problem is that the shared system executors are getting out of whack when restarting the React Native application without restarting the underlying Android Main Activity.

And (sorry to repeat myself), arbitrarily swapping to parallel execution could definitely result in unanticipated knock-on effects for some app developers.

[slegaitis]

I figured a sort of reason why this is happening. If You click CTRL + S or CMD + S to save the file twice this will trigere, Live Reload. Sometimes if you click ctrl + s or cmd + s once Live Reload doesn't work, and if you click it second time almost immediately after first attempt, I'm able to replicate the issue.

Workaround that works for me for now is to close the app, go to Simulator/ Real device menu and open it again. Then AsyncStorage works again.

[Pastromhaug]

I still have this problem in 0.55.4

[tntdarken]

I think I have found a possible solution to this problem. I hope it helps:

Component:
componentWillMount(){
this.storageClass.getStorage().then( retorno => this.setState(retorno));
}

storageClass:
async getStorage(){
data = await AsyncStorage.getAllKeys();
return AsyncStorage.multiGet(data).then( data => {
let retorno = {};
data.forEach( async x => {
key = x[0];
value = x[1];
retorno[key] = value;
});
return retorno;
});
}

[leakim34]

Hi,

same for me here and very frustrating because asyncstorage is as major dep of Apollo Client when we want to cache the app.

Please we need a fix for this ... opened since May 2K17 !

Thanks everyone

[leandrohsilveira]

As @slegaitis said, it is something related to Live Reload option.
It works when the app is opened with the Live Reload disabled.

Fortunately, it also works with "Reload" option and the VSCode Extension called React Native Tools has a shortcut to use this option using CTRL + Shift + P > React-Native: Reload App.

[bstst]

It does not work with remote debugging enabled though, no matter if Live Reload is enabled.

[haggholm]

Any news on this? It looks to me like the CI test was failing for reasons unrelated to the PR; maybe it just needs to re-run? It would be great to see this in the next RN release…

[ITsolution-git]

Hello.
When I use debugger, asyncstorage promise seems to be resolved.
But it does not work without debugging, and it does not work on testflight deployed.
The asyncstorage promise never resolves, and even the app freezes and abort after about 5-10 seconds.

I placed an loader till the promise resolves but loader is not rendered and seems like the app refused to render anymore.

Please help anyone.
Thanks

[jsdario]

Closing as the bug is solved and this PR is over a year old

[Waqas-Jani]

i have the same issue .. asynstroage not working on Android version 24+(7+).. any one has solutaion??

[ollyfg]

@Waqas-Jani Can you tell us what version of React Native you are running? Are you using the older built-in AsyncStorage or the newer community one (@react-native-community/async-storage)? Are you seeing any related errors, either through JS debugging or logCat?

Either way, this bug has been solved and you might be seeing a different one. I would recommend opening a new issue on https://github.com/react-native-community/async-storage