-
Notifications
You must be signed in to change notification settings - Fork 1k
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 Rust values getting GC'd while still borrowed and not getting GC'd if created via. constructor #3940
Open
Liamolucko
wants to merge
12
commits into
rustwasm:main
Choose a base branch
from
Liamolucko:gc-fixes
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Commits on Apr 27, 2024
-
Configuration menu - View commit details
-
Copy full SHA for 070a4a8 - Browse repository at this point
Copy the full SHA 070a4a8View commit details -
Force tests to run sequentially
At first I tried fixing them so that they didn't need to (giving each of them their own `dropCount`), but running multiple GCs in parallel seems to be flaky.
Configuration menu - View commit details
-
Copy full SHA for 16bfc43 - Browse repository at this point
Copy the full SHA 16bfc43View commit details -
Configuration menu - View commit details
-
Copy full SHA for e6551c1 - Browse repository at this point
Copy the full SHA e6551c1View commit details -
Configuration menu - View commit details
-
Copy full SHA for cad34fc - Browse repository at this point
Copy the full SHA cad34fcView commit details
Commits on Apr 28, 2024
-
Fix exported Rust types being GC'd while still borrowed
I also discovered and fixed an extra bug while working on this, which was that `LongRefFromWasmAbi` wasn't getting used for `self`: this bug didn't cause any problems before, because the only type that had a different `LongRefFromWasmAbi` impl than its `RefFromWasmAbi` impl was `JsValue` which can't be the type of `self`. It became a problem here because I made the new `LongRefFromWasmAbi` impl for exported Rust types clone the `Rc`, whereas the `RefFromWasmAbi` impl doesn't because garbage collection can't occur during the synchronous call that the value has to live for. I might actually change it so that both of the impls behave like the current `LongRefFromWasmAbi` impl, though: cloning an `Rc` isn't expensive and so having the second different impl just makes things more complicated for no good reason. I just left it in this commit as explanation for how I discovered the `LongRefFromWasmAbi` issue.
Configuration menu - View commit details
-
Copy full SHA for 3bd0f3c - Browse repository at this point
Copy the full SHA 3bd0f3cView commit details -
Configuration menu - View commit details
-
Copy full SHA for 7fdbe95 - Browse repository at this point
Copy the full SHA 7fdbe95View commit details -
Configuration menu - View commit details
-
Copy full SHA for d919842 - Browse repository at this point
Copy the full SHA d919842View commit details -
Get rid of outdated
borrow_mut
Now that borrowing a Rust value always clones its `Rc`, `Rc::into_inner` is a sufficient check that the value isn't borrowed.
Configuration menu - View commit details
-
Copy full SHA for 45c4444 - Browse repository at this point
Copy the full SHA 45c4444View commit details -
For some reason I was getting errors before without it, but that seems to be fixed now. (It's probably something to do with having removed the `borrow_mut`, but that only takes `&self`, so I still don't get it.)
Configuration menu - View commit details
-
Copy full SHA for c72cbde - Browse repository at this point
Copy the full SHA c72cbdeView commit details -
Configuration menu - View commit details
-
Copy full SHA for db0c540 - Browse repository at this point
Copy the full SHA db0c540View commit details -
Configuration menu - View commit details
-
Copy full SHA for f992a18 - Browse repository at this point
Copy the full SHA f992a18View commit details -
Configuration menu - View commit details
-
Copy full SHA for fa78ca6 - Browse repository at this point
Copy the full SHA fa78ca6View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.