Skip to content

Add a new mrb_fiber_new() with MRB_API #6097

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.

Sign up for GitHub

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

Merged
matz merged 1 commit into from
Nov 7, 2023
+81 −59
Merged

Add a new mrb_fiber_new() with MRB_API #6097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/mruby.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,13 @@ MRB_API mrb_bool mrb_func_basic_p(mrb_state *mrb, mrb_value obj, mrb_sym mid, mr
/* obsolete function(s); will be removed */
#define mrb_int(mrb, val) mrb_as_int(mrb, val)

/**
* Create a new Fiber from proc object
*
* Implemented in mruby-fiber
*/
MRB_API mrb_value mrb_fiber_new(mrb_state *mrb, const struct RProc *proc);

/**
* Resume a Fiber
*
Expand Down
133 changes: 74 additions & 59 deletions mrbgems/mruby-fiber/src/fiber.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <mruby/class.h>
#include <mruby/error.h>
#include <mruby/proc.h>
#include <mruby/presym.h>

#define fiber_ptr(o) ((struct RFiber*)mrb_ptr(o))

Expand All @@ -11,6 +12,66 @@
/* copied from vm.c */
#define CINFO_RESUMED 3

static mrb_value
fiber_init_fiber(mrb_state *mrb, struct RFiber *f, const struct RProc *p)
{
static const struct mrb_context mrb_context_zero = { 0 };
struct mrb_context *c;
mrb_callinfo *ci;
size_t slen;

if (f->cxt) {
mrb_raise(mrb, E_RUNTIME_ERROR, "cannot initialize twice");
}
if (MRB_PROC_CFUNC_P(p)) {
mrb_raise(mrb, E_FIBER_ERROR, "tried to create Fiber from C defined method");
}

c = (struct mrb_context*)mrb_malloc(mrb, sizeof(struct mrb_context));
*c = mrb_context_zero;
f->cxt = c;

/* initialize VM stack */
slen = FIBER_STACK_INIT_SIZE;
if (p->body.irep->nregs > slen) {
slen += p->body.irep->nregs;
}
c->stbase = (mrb_value*)mrb_malloc(mrb, slen*sizeof(mrb_value));
c->stend = c->stbase + slen;

{
mrb_value *p = c->stbase;
mrb_value *pend = c->stend;

while (p < pend) {
SET_NIL_VALUE(*p);
p++;
}
}

/* copy receiver from a block */
c->stbase[0] = mrb->c->ci->stack[0];

/* initialize callinfo stack */
c->cibase = (mrb_callinfo*)mrb_calloc(mrb, FIBER_CI_INIT_SIZE, sizeof(mrb_callinfo));
c->ciend = c->cibase + FIBER_CI_INIT_SIZE;
c->ci = c->cibase;

/* adjust return callinfo */
ci = c->ci;
mrb_vm_ci_target_class_set(ci, MRB_PROC_TARGET_CLASS(p));
mrb_vm_ci_proc_set(ci, p);
mrb_field_write_barrier(mrb, (struct RBasic*)f, (struct RBasic*)p);
ci->stack = c->stbase;
ci[1] = ci[0];
c->ci++; /* push dummy callinfo */

c->fib = f;
c->status = MRB_FIBER_CREATED;

return mrb_obj_value(f);
}

/*
* call-seq:
* Fiber.new{...} -> obj
Expand Down Expand Up @@ -66,67 +127,9 @@
static mrb_value
fiber_init(mrb_state *mrb, mrb_value self)
{
static const struct mrb_context mrb_context_zero = { 0 };
struct RFiber *f = fiber_ptr(self);
struct mrb_context *c;
struct RProc *p;
mrb_callinfo *ci;
mrb_value blk;
size_t slen;

mrb_get_args(mrb, "&!", &blk);

if (f->cxt) {
mrb_raise(mrb, E_RUNTIME_ERROR, "cannot initialize twice");
}
p = mrb_proc_ptr(blk);
if (MRB_PROC_CFUNC_P(p)) {
mrb_raise(mrb, E_FIBER_ERROR, "tried to create Fiber from C defined method");
}

c = (struct mrb_context*)mrb_malloc(mrb, sizeof(struct mrb_context));
*c = mrb_context_zero;
f->cxt = c;

/* initialize VM stack */
slen = FIBER_STACK_INIT_SIZE;
if (p->body.irep->nregs > slen) {
slen += p->body.irep->nregs;
}
c->stbase = (mrb_value*)mrb_malloc(mrb, slen*sizeof(mrb_value));
c->stend = c->stbase + slen;

{
mrb_value *p = c->stbase;
mrb_value *pend = c->stend;

while (p < pend) {
SET_NIL_VALUE(*p);
p++;
}
}

/* copy receiver from a block */
c->stbase[0] = mrb->c->ci->stack[0];

/* initialize callinfo stack */
c->cibase = (mrb_callinfo*)mrb_calloc(mrb, FIBER_CI_INIT_SIZE, sizeof(mrb_callinfo));
c->ciend = c->cibase + FIBER_CI_INIT_SIZE;
c->ci = c->cibase;

/* adjust return callinfo */
ci = c->ci;
mrb_vm_ci_target_class_set(ci, MRB_PROC_TARGET_CLASS(p));
mrb_vm_ci_proc_set(ci, p);
mrb_field_write_barrier(mrb, (struct RBasic*)mrb_obj_ptr(self), (struct RBasic*)p);
ci->stack = c->stbase;
ci[1] = ci[0];
c->ci++; /* push dummy callinfo */

c->fib = f;
c->status = MRB_FIBER_CREATED;

return self;
return fiber_init_fiber(mrb, fiber_ptr(self), mrb_proc_ptr(blk));
}

static struct mrb_context*
Expand Down Expand Up @@ -457,6 +460,18 @@ fiber_current(mrb_state *mrb, mrb_value self)
return mrb_obj_value(mrb->c->fib);
}

MRB_API mrb_value
mrb_fiber_new(mrb_state *mrb, const struct RProc *p)
{
struct RClass *c = mrb_class_get_id(mrb, MRB_SYM(Fiber));
if (MRB_INSTANCE_TT(c) != MRB_TT_FIBER) {

@matz matz Nov 6, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When MRB_INSTANCE_TT(c) != MRB_TT_FIBER, MRB_OBJ_ALLOC will cause TypeError.
Do we really need this check?

@dearblue dearblue Nov 6, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I consider it necessary.

% cat 1.c
#include <mruby.h>

int
main(int argc, char *argv[])
{
  mrb_state *mrb = mrb_open();
  struct RFiber *o = MRB_OBJ_ALLOC(mrb, MRB_TT_FIBER, mrb->object_class);
  mrb_p(mrb, mrb_obj_value(o));
  mrb_close(mrb);

  return 0;
}

% $(bin/mruby-config --cc --cflags --ldflags --libs) 1.c && ./a.out
#<Object:0x823fcf720>

This is because the instance type tag of the Object class is MRB_TT_FALSE and the ttype parameter of mrb_obj_alloc() is rarely checked in this case.

mruby/src/gc.c

Lines 477 to 490 in 9e50d67

tt = MRB_INSTANCE_TT(cls);
if (tt != MRB_TT_FALSE &&
tt != MRB_TT_UNDEF &&
ttype != MRB_TT_SCLASS &&
ttype != MRB_TT_ICLASS &&
ttype != MRB_TT_ENV &&
ttype != MRB_TT_BIGINT &&
ttype != tt) {
mrb_raisef(mrb, E_TYPE_ERROR, "allocation failure of %C", cls);
}
}
if (ttype <= MRB_TT_FREE) {
mrb_raisef(mrb, E_TYPE_ERROR, "allocation failure of %C (type %d)", cls, (int)ttype);
}

Or is it better to take the same approach as Random?
9b9424f#diff-bf9e7692602cba54ba62bd71b76c439be9c9b71c6bb8d51ab996fc91cf1ee807

@matz matz Nov 6, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to review the condition in mrb_obj_alloc().

Besides that, I don't think we should take Random approach. Random uses TT_ISTRUCT type that may be used by other classes. In contrast, TT_FIBER is only used by Fiber class.

@dearblue dearblue Nov 6, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this will help, but I have an experimental branch that I am suspending in the middle of a work in progress to make mrb_obj_alloc() more rigorous.
master...dearblue:mruby:wip/alloc-strict
The reason I am suspending it is because of compatibility concerns and at least my mruby-lz4 needs to be modified.

@matz matz Nov 7, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will merge this PR as it is. Then I will try to update mrb_obj_allocate.

mrb_raise(mrb, E_TYPE_ERROR, "wrong Fiber class");
}

struct RFiber *f = MRB_OBJ_ALLOC(mrb, MRB_TT_FIBER, c);
return fiber_init_fiber(mrb, f, p);
}

void
mrb_mruby_fiber_gem_init(mrb_state* mrb)
{
Expand Down