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 unexpected free of root fiber object and add correct Fiber#== due to it. #1865

Closed
wants to merge 2 commits into from

Conversation

take-cheeze
Copy link
Contributor

Marking fib field of mrb_context is an alternative of this fix.

@matz
Copy link
Member

matz commented Mar 17, 2014

I am not sure what problem are you going to solve by this PR.
Does creating fiber object each time solve anything?
Is there any program that reproduce the problem?

@take-cheeze
Copy link
Contributor Author

After running the following script in root context Fiber.current in root context will return MRB_TT_FREE object since fib field of mrb_context won't be marked.(The assertion I added to check_fiber function should fail when checking alive? method)

 root = Fiber.current
 root = nil
 GC.start

Marking fib field seems enough to solve this problem.
Though creating any number of Fiber object from mrb->root_c is allowed(since it won't be freed until mrb_close is called) so I added Fiber#== method.
If you think marking fib is is enough please close this.

@matz matz closed this in 9201af7 Mar 18, 2014
matz added a commit that referenced this pull request Mar 18, 2014
matz added a commit that referenced this pull request Mar 18, 2014
@take-cheeze take-cheeze deleted the fiber_eq branch March 19, 2014 20:25
mattn pushed a commit to mattn/mruby that referenced this pull request Sep 11, 2015
mattn pushed a commit to mattn/mruby that referenced this pull request Sep 11, 2015
mattn pushed a commit to mattn/mruby that referenced this pull request Sep 11, 2015
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