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
Allow recycling fibers by GC if not referenced directly #6253
Conversation
`mrb_env_unshare()` calls `mrb_realloc_simple()` and follows `mrb_full_gc()` to avoid an infinite loop where `mrb_env_unshare()` is called again. This does not occur at this time, but may occur in subsequent patches.
The patch assumes that `struct REnv::cxt` only performs checks with the `OP_BREAK` and `OP_RETURN_BLK` instructions, and does not reference the entity. Therefore, by changing to a weak reference, it is possible to collect fibers that are no longer directly referenced while in the suspended state. However, we need to detach the living env objects that remain in the call stack of the fiber. So, in effect, it involves a revert of following commits. - commit a3365d8 - commit 57ffa1c Examples of the effects of change are shown below. Note that it was built with `rake MRUBY_CONFIG=host-debug`. ```ruby f = Fiber.new { (x, y, z) = "X", "Y", "Z"; Fiber.yield -> { [x, y, z] } } g = f.resume GC.start p ObjectSpace.memsize_of_all # => 59532 g.call # => ["X", "Y", "Z"] f = nil GC.start ObjectSpace.memsize_of_all # BEFORE => 59532 # AFTER => 58044 g.call # => ["X", "Y", "Z"] ```
@@ -773,7 +772,20 @@ obj_free(mrb_state *mrb, struct RBasic *obj, mrb_bool end) | |||
{ | |||
struct mrb_context *c = ((struct RFiber*)obj)->cxt; | |||
|
|||
if (c != mrb->root_c) { | |||
if (c && c != mrb->root_c) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since mrb->root_c
is never NULL, I don't think the c &&
check is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your review.
I checked and it seems that omitting the NULL
check on the c
variable causes a crash due to a "null pointer dereference".
% lldb18 -- bin/mruby -e 'Fiber.allocate; GC.start'
(lldb) target create "bin/mruby"
Current executable set to '/var/tmp/mruby/bin/mruby' (x86_64).
(lldb) settings set -- target.run-args "-e" "Fiber.allocate; GC.start"
(lldb) r
Process 90188 launched: '/var/tmp/mruby/bin/mruby' (x86_64)
Process 90188 stopped
* thread #1, name = 'mruby', stop reason = signal SIGSEGV: address not mapped to object (fault address: 0x30)
frame #0: 0x0000000000413264 mruby`obj_free(mrb=0x000008d4df809000, obj=0x000008d4df82ad70, end=false) at gc.c:776:24
773 struct mrb_context *c = ((struct RFiber*)obj)->cxt;
774
775 if (c != mrb->root_c) {
-> 776 if (!end && c->status != MRB_FIBER_TERMINATED) {
777 mrb_callinfo *ci = c->ci;
778 mrb_callinfo *ce = c->cibase;
779
mrb_free_context()
is "NULL safe", so there is no problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, thank you for the confirmation. My bad.
if (MRB_ENV_ONSTACK_P(e) && e->cxt && e->cxt->fib) { | ||
mrb_gc_mark(mrb, (struct RBasic*)e->cxt->fib); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was checking again and found a period when this section did not exist.
% git log -L627,640:src/gc.c 22518b5b789293e7ddad7a94def44039937d8e8c
There is no mention of a commit message, but it corresponds to a revert of commit b6598e0.
The patch assumes that
struct REnv::cxt
only performs checks with theOP_BREAK
andOP_RETURN_BLK
instructions, and does not reference the entity.Therefore, by changing to a weak reference, it is possible to collect fibers that are no longer directly referenced while in the suspended state.
However, we need to detach the living env objects that remain in the call stack of the fiber.
So, in effect, it involves a revert of following commits.
Examples of the effects of change are shown below.
Note that it was built with
rake MRUBY_CONFIG=host-debug
.In addition, it also takes the necessary precautions to prevent the possibility of infinite loops with this change.