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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect self and CompiledCode from GC before handling arguments #3626

Conversation

ahmadsherif
Copy link
Member

When arguments are keywords, GenericArguments::call will call Rubinius::Runtime.keyword_object?, which results in allocating some objects. When the young zone is full, garbage collection is run, which could lead to forwarding the self and/or the CompiledCode objects without updating their references, resulting in a faulty behavior or crashes like in #3540.

However, I'm not sure about the solution, so a feedback would be appreciated 馃槂 .

When arguments are keywords, `GenericArguments::call` will call
`Rubinius::Runtime.keyword_object?`, which results in allocating some
objects. When the young zone is full, garbage collection is run, which
could lead to forwarding the self and/or the CompiledCode objects
without updating their references, resulting in a faulty behavior or
crashes like in rubinius#3540.

Fixes rubinius#3540
@brixen
Copy link
Member

brixen commented Mar 6, 2016

@ahmadsherif thanks for looking into this. The situation you identified is a big problem across many parts of the codebase and is the reason I started on the work in the stw branch. That branch makes all allocation paths GC-safe (ie the GC will never run during allocation), among other things.

Since this is only one of several issues and I'm really close to merging stw, I'd like to hold off on these changes.

@ahmadsherif
Copy link
Member Author

@brixen No problem, thanks you for clarifying 馃槂 .

@ahmadsherif ahmadsherif closed this Mar 6, 2016
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.

None yet

2 participants