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: resolve PromiseProxy context memory leak #193

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

edusperoni
Copy link
Collaborator

@edusperoni edusperoni commented Feb 14, 2023

This will cleanup references to resolve and reject after they're called once.

I'm not sure if this will solve the issue for never resolving promises, but in my tests it resolved the issues in @nativescript/core where ImageSource would leak after loading an image from a url.

To go deeper into the issue, this happened because the promise would keep a reference to PromiseProxy's resolve/reject functions, which started to become a problem when this function would then add a native callback, which would keep a reference to the full context, keeping the resolve/reject always in memory (and usually also the promise itself).

Ultimately this created a circular reference with the reactions_or_result GC root that didn't go away until we removed the reference to the original resove/reject functions.

Having trouble following the GC logic? Me too.

Might fix #100. Although I can't reproduce it with the repro code in that issue

@cla-bot cla-bot bot added the cla: yes label Feb 14, 2023
@NathanWalker NathanWalker merged commit 21de81d into main Feb 15, 2023
@NathanWalker NathanWalker deleted the fix/promise-memory-leak branch February 15, 2023 03:48
caitp pushed a commit to caitp/ns-v8ios-runtime that referenced this pull request Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Promise.resolve and Promise.reject causes memory leak on iOS.
2 participants