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 crash when accessing weak reference #36099

Closed
wants to merge 1 commit into from
Closed

Conversation

scarlac
Copy link
Contributor

@scarlac scarlac commented Feb 8, 2023

Crash with message uiManagerWillPerformMounting: Attempted to dereference garbage pointer <address>

image

Summary

We are seeing multiple crashes in production apps where uiManagerWillPerformMounting is mentioned in the context of a PHPicker controller being shown. While it's not clear how to reliable reproduce the issue, after reviewing the culprit, it does seem like an issue that a weak reference hash map is being used to access shadow views. Between being added to the map and being called they could be freed, causing this crash.

Changelog

[IOS] [FIXED] - Fixed "uiManagerWillPerformMounting" native crash while another UIViewController is active.

Test Plan

Code was NOT tested. Reproducibility is an issue, and I was not able to gather any feedback on this proposal outside of a GH PR. I would like some feedback before we merge.
However, it's clear that weak references should always be checked so it seems fine to do this.

Crash with message "uiManagerWillPerformMounting: Attempted to dereference garbage pointer <address>"
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 8, 2023
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,475,498 -180
android hermes armeabi-v7a 7,796,965 -363
android hermes x86 8,951,524 -203
android hermes x86_64 8,809,332 -355
android jsc arm64-v8a 9,113,276 -186
android jsc armeabi-v7a 8,309,955 -356
android jsc x86 9,164,867 -195
android jsc x86_64 9,423,750 -357

Base commit: 2e3f55a
Branch: main

Copy link
Member

@javache javache left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your changes will not resolve the issue at hand.

In Objective-C it's safe to call methods on nil objects, they will just become no-ops. NSHashTable will also filter out weak elements that have become deallocated when iterating.

If the pointer is a garbage value like 0xC, an if check will not filter then out.

@scarlac scarlac closed this Feb 9, 2023
@scarlac
Copy link
Contributor Author

scarlac commented Feb 9, 2023

Thanks. Not sure how to fix the issue of garbage pointer then but you are probably right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants