Skip to content

[CP] [vm/ffi] Fix FfiTrampolineData GC bug #50695

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

Closed
dcharkes opened this issue Dec 12, 2022 · 7 comments
Closed

[CP] [vm/ffi] Fix FfiTrampolineData GC bug #50695

dcharkes opened this issue Dec 12, 2022 · 7 comments
Assignees
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta

Comments

@dcharkes
Copy link
Contributor

Commit(s) to merge

e3e355e

Target

beta

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/275060

Issue Description

GC crashes with FFI trampolines, happening on all platforms in JIT mode.

What is the fix

Make the GC visit the c_signature field again.

Why cherry-pick

Users are hitting this:

Risk

low

Issue link(s)

#50678

Extra Info

cc @mkustermann

@dcharkes dcharkes added the cherry-pick-review Issue that need cherry pick triage to approve label Dec 12, 2022
@itsjustkevin
Copy link
Contributor

@mkustermann could you take a look at this cherry-pick request?

@mkustermann
Copy link
Member

@mkustermann could you take a look at this cherry-pick request?

@itsjustkevin LGTM

@itsjustkevin itsjustkevin added merge-to-beta cherry-pick-approved Label for approved cherrypick request labels Dec 14, 2022
@itsjustkevin
Copy link
Contributor

@dcharkes cherry-pick is approved, can you go ahead and push the gerrit cl?

@dcharkes
Copy link
Contributor Author

I'm unfamiliar with the new process, you mean +2 in Gerrit?

I'd need a +1 on the CL.

@athomas
Copy link
Member

athomas commented Dec 15, 2022 via email

@mit-mit mit-mit added the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label Jan 4, 2023
@whesse whesse removed the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label Jan 4, 2023
@mit-mit
Copy link
Member

mit-mit commented Jan 4, 2023

@dcharkes are you still working on this?

@dcharkes
Copy link
Contributor Author

dcharkes commented Jan 5, 2023

This was merged back in december.

@dcharkes dcharkes closed this as completed Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta
Projects
None yet
Development

No branches or pull requests

7 participants