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

fix(android): fix headless crash with null ReactContext #1168

Merged
merged 5 commits into from
Dec 16, 2024

Conversation

mikehardy
Copy link
Collaborator

very tricky to make sure react-native is initialized during headlessJS startup so you don't beat it in the race to asking for a ReactContext before it exists

  • startup is event-driven so you have to attach listeners
  • startup has to run on main thread so you have to post runnables
  • multiple tasks may come in while starting up, you must queue / drain
  • startup objects / methods differ across react-native new / old arch

get all that right, and you'll have a valid ReactContext, without dropping any tasks that needed it before react-native was initialized

Previously, we were attempting to use a null ReactContext, resulting in errors from upstream headless services complaining about the context already being destroyed

Fixes #266

very tricky to make sure react-native is initialized during
headlessJS startup so you don't beat it in the race to asking
for a ReactContext before it exists

- startup is event-driven so you have to attach listeners
- startup has to run on main thread so you have to post runnables
- multiple tasks may come in while starting up, you must queue / drain
- startup objects / methods differ across react-native new / old arch

get all that right, and you'll have a valid ReactContext, without
dropping any tasks that needed it before react-native was initialized
@mikehardy mikehardy added the pending-merge Waiting on CI or question responses to merge, but otherwise ready label Dec 15, 2024
Copy link

codecov bot commented Dec 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.08%. Comparing base (77e72bd) to head (29a0745).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1168   +/-   ##
=======================================
  Coverage   77.08%   77.08%           
=======================================
  Files          32       32           
  Lines        1727     1727           
  Branches      556      556           
=======================================
  Hits         1331     1331           
  Misses        395      395           
  Partials        1        1           

otherwise react-native complains a lot in the log
now there is a button that will create a trigger notification
for +15 secs, allowing time to kill the app. Then a background
handler should tell you that you got a notification in the background
this should have been in index.js so it could be registered as
a background handler prior to AppRegistry registration of the app
@mikehardy mikehardy merged commit 7359069 into main Dec 16, 2024
14 checks passed
@mikehardy mikehardy deleted the @mikehardy/fix-another-headless-crash branch December 16, 2024 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant