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

Hack patch for testing OTA update crash behavior #4942

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Conversation

haileyok
Copy link
Contributor

Why

There's a crash that's happening when reloadAsync() gets called by expo-updates. This is coming from attempting to destructure a null pointer, whenever removing all of the SharedObjects in SharedObjectRegistry.clear().

A little digging shows that the deinit block of VideoPlayer runs after clear() get's called. However, right now clear() calls dict.removeAll() asynchronously inside of a separate queue from the main one. This is a small test to see if this helps to alievate the crashes. I don't consider this a production fix.

Test Plan

Merge, let it build, and monitor for OTA crashes as we have been internally.

Copy link

render bot commented Aug 15, 2024

Copy link

The Pull Request introduced fingerprint changes against the base commit:

Fingerprint diff
[{"type":"dir","filePath":"node_modules/expo-modules-core","reasons":["expoAutolinkingIos","expoAutolinkingAndroid"],"hash":"9df71ac8c4a237da3d991ee0a66316611d55e78d"},{"type":"dir","filePath":"patches","reasons":["patchPackage"],"hash":"e2cee35d425489380293c880e297c36a4c411d90"}]

Generated by PR labeler 🤖

@haileyok haileyok merged commit f3b57dd into main Aug 15, 2024
5 of 6 checks passed
@haileyok haileyok deleted the hailey/test-crash-hack branch August 15, 2024 19:03
@haileyok
Copy link
Contributor Author

we do want to revert this prior to cutting a release. added a note to release tracker.

estrattonbailey added a commit that referenced this pull request Aug 21, 2024
* origin/main: (50 commits)
  Add `list hidden` screen (#4958)
  Expose more methods, support disabled items (#4954)
  Expose more props from button (#4953)
  Fix orphaned feed slices, handle blocks (#4944)
  Tweak `expo-modules-core` hack patch (#4955)
  [Experiment] Always show bottom bar (#4946)
  Revert "[Video] Download videos" (#4945)
  Move global "Sign out" out of the current account row (#4941)
  Hack patch for testing OTA update crash behavior (#4942)
  [Video] Download videos (#4886)
  swap control files (#4936)
  [Embed] Starter pack embed embed (#4935)
  [Video] set audio category to ambient every time a new player is made (#4934)
  Add `/live/` to supported YouTube embed URLs (#4932)
  [Video] Try/catch video play/pause (#4930)
  Don't kick to login screen on network error (#4911)
  Remove .withProxy() calls (#4929)
  [Video] Audio duck off main thread (#4926)
  subclass agent to add setPersistSessionHandler (#4928)
  [Video] Fix crash when switching tabs (#4925)
  ...
haileyok added a commit that referenced this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants