Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flush RN task queue with invokeAsync #4389

Merged
merged 6 commits into from Apr 27, 2022

Conversation

tomduncalf
Copy link
Contributor

@tomduncalf tomduncalf commented Mar 2, 2022

What, How & Why?

This closes #3571, #4389

React Native has its own JS "microtask" queue, implemented in https://github.com/facebook/react-native/blob/main/Libraries/Core/Timers/JSTimers.js. Any asynchronous work, e.g. setTimeout or setImmediate, is added to this queue, and the queue is flushed whenever a message is sent from React Native's native core to JS: https://github.com/facebook/react-native/blob/main/Libraries/BatchedBridge/MessageQueue.js#L108.

However, our native to JS messages are not being passed via this abstraction - instead, we hook directly into the JS engine. This means that React Native is not aware that when Realm has done some async work, we might need to update the UI (and therefore flush the task queue), and this can result in Realm-related UI updates not showing until some action is taken which causes React Native to send a message from the core to JS (e.g. touching the screen), which flushes this task queue, resolving pending promises and updating the UI.

After some discussion and research (e.g. facebook/react-native#33006), it was determined that exposing and calling the invokeAsync method from the React Native CallInvoker was the most reliable way of triggering a flush of this queue without having to modify React Native internals to directly expose their flush method – the other approaach we tried (exposing an internal React Native JS function to flush the queue, see #4330) was not 100% reliable.

We pass a lambda which calls this method (debounced, so we don't call it multiple times if it has not yet been invoked asynchronously) down from our RN module initialization code into the JSC code, which can then call this method whenever we make call from C++ to JS.

This issue could not be reproduced on Android, and the current Android JSC "trampoline" makes it hard to work out how to call into invokeAsync in the same way. However, it should be possible to port this fix to the Hermes branch for Android as that code has been reworked.

☑️ ToDos

  • 📝 Changelog entry
  • 📝 Compatibility label is updated or copied from previous entry
  • 🚦 Tests
  • 📱 Check the React Native/other sample apps work if necessary
  • 📝 Public documentation PR created or is not necessary
  • 💥 Breaking label has been applied or is not necessary

If this PR adds or changes public API's:

  • typescript definitions file is updated
  • jsdoc files updated
  • Chrome debug API is updated if API is available on React Native

@cla-bot cla-bot bot added the cla: yes label Mar 2, 2022
@tomduncalf tomduncalf changed the title TTd/fix rn queue issue invoke async Flush RN task queue with invokeAsync Mar 2, 2022
@tomduncalf tomduncalf force-pushed the td/fix-rn-queue-issue-invoke-async branch from 86c375a to 34fcb41 Compare April 19, 2022 10:24
…by calling jsCallInvoker->invokeAsync.

React Native has its own JS "microtask" queue, implemented in https://github.com/facebook/react-native/blob/main/Libraries/Core/Timers/JSTimers.js. Any asynchronous work, e.g. `setTimeout` or `setImmediate`, is added to this queue, and the queue is flushed whenever a message is sent from React Native's native core to JS: https://github.com/facebook/react-native/blob/main/Libraries/BatchedBridge/MessageQueue.js#L108.

However, our native to JS messages are not being passed via this abstraction - instead, we hook directly into the JS engine. This means that React Native is not aware that when Realm has done some async work, we might need to update the UI (and therefore flush the task queue), and this can result in Realm-related UI updates not showing until some action is taken which causes React Native to send a message from the core to JS (e.g. touching the screen), which flushes this task queue, resolving pending promises and updating the UI.

This commit calls the React Native jsCallInvoker->invokeAsync method, which internally flushes the task queue. As this method is async, we wait for any current pending invocation to complete before triggering another one (using a flag, so the call is "debounced")
@tomduncalf tomduncalf force-pushed the td/fix-rn-queue-issue-invoke-async branch from 34fcb41 to c7dccf5 Compare April 19, 2022 11:20
@tomduncalf tomduncalf force-pushed the td/fix-rn-queue-issue-invoke-async branch from aabc001 to 0e1da78 Compare April 26, 2022 11:11
@tomduncalf tomduncalf force-pushed the td/fix-rn-queue-issue-invoke-async branch from 0e1da78 to 1a1b6e3 Compare April 26, 2022 11:15
@tomduncalf tomduncalf marked this pull request as ready for review April 26, 2022 12:01
@tomduncalf tomduncalf merged commit 9289e11 into master Apr 27, 2022
@tomduncalf tomduncalf deleted the td/fix-rn-queue-issue-invoke-async branch April 27, 2022 08:44
@tomduncalf tomduncalf mentioned this pull request Jul 14, 2022
11 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Promises / event loop stops working until UI is tapped
2 participants