Skip to content

Commit

Permalink
Merge pull request #701 from h2o/kazuho/issues/699
Browse files Browse the repository at this point in the history 
fix and reduce the risk of GC-arena related leaks
  • Loading branch information
@kazuho
kazuho committed Jan 14, 2016
2 parents f0e3af1 + 1b3dd14 commit e49fdd7
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 26 deletions.
2 changes: 1 addition & 1 deletion include/h2o/mruby_.h
View file
Expand Up @@ -143,7 +143,7 @@ void h2o_mruby_define_callback(mrb_state *mrb, const char *name, int id);
mrb_value h2o_mruby_create_data_instance(mrb_state *mrb, mrb_value class_obj, void *ptr, const mrb_data_type *type);
mrb_value h2o_mruby_compile_code(mrb_state *mrb, h2o_mruby_config_vars_t *config, char *errbuf);
h2o_mruby_handler_t *h2o_mruby_register(h2o_pathconf_t *pathconf, h2o_mruby_config_vars_t *config);
void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, mrb_value input, int gc_arena, int *is_delegate);
void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, mrb_value input, int *is_delegate);
mrb_value h2o_mruby_each_to_array(h2o_mruby_context_t *handler_ctx, mrb_value src);
int h2o_mruby_iterate_headers(h2o_mruby_context_t *handler_ctx, mrb_value headers,
int (*cb)(h2o_mruby_context_t *, h2o_iovec_t, h2o_iovec_t, void *), void *cb_data);
Expand Down
33 changes: 11 additions & 22 deletions lib/handler/mruby.c
View file
Expand Up @@ -195,7 +195,7 @@ static mrb_value build_constants(mrb_state *mrb, const char *server_name, size_t
mrb_value ary = mrb_ary_new_capa(mrb, H2O_MRUBY_NUM_CONSTANTS);
mrb_int i;

int arena = mrb_gc_arena_save(mrb);
int gc_arena = mrb_gc_arena_save(mrb);

{
h2o_mem_pool_t pool;
Expand All @@ -213,7 +213,6 @@ static mrb_value build_constants(mrb_state *mrb, const char *server_name, size_t
FREEZE_STRING(lit);
mrb_ary_set(mrb, ary, i, lit);
}
mrb_gc_arena_restore(mrb, arena);
}
h2o_mem_clear_pool(&pool);
}
Expand All @@ -223,7 +222,6 @@ static mrb_value build_constants(mrb_state *mrb, const char *server_name, size_t
mrb_value lit = (value); \
FREEZE_STRING(lit); \
mrb_ary_set(mrb, ary, idx, lit); \
mrb_gc_arena_restore(mrb, arena); \
} while (0)
#define SET_LITERAL(idx, str) SET_STRING(idx, mrb_str_new_lit(mrb, str))

Expand Down Expand Up @@ -302,6 +300,7 @@ static mrb_value build_constants(mrb_state *mrb, const char *server_name, size_t
"end");
h2o_mruby_assert(mrb);

mrb_gc_arena_restore(mrb, gc_arena);
return ary;
}

Expand Down Expand Up @@ -569,7 +568,7 @@ static int on_req(h2o_handler_t *_handler, h2o_req_t *req)
{
h2o_mruby_handler_t *handler = (void *)_handler;
h2o_mruby_context_t *handler_ctx = h2o_context_get_handler_context(req->conn->ctx, &handler->super);
int arena = mrb_gc_arena_save(handler_ctx->mrb);
int gc_arena = mrb_gc_arena_save(handler_ctx->mrb);

h2o_mruby_generator_t *generator = h2o_mem_alloc_shared(&req->pool, sizeof(*generator), on_generator_dispose);
generator->super.proceed = NULL;
Expand All @@ -582,14 +581,15 @@ static int on_req(h2o_handler_t *_handler, h2o_req_t *req)
mrb_value env = build_env(generator);

int is_delegate = 0;
h2o_mruby_run_fiber(generator, generator->ctx->proc, env, arena, &is_delegate);
h2o_mruby_run_fiber(generator, generator->ctx->proc, env, &is_delegate);

mrb_gc_arena_restore(handler_ctx->mrb, gc_arena);
if (is_delegate)
return -1;
return 0;
}

static void send_response(h2o_mruby_generator_t *generator, mrb_int status, mrb_value resp, int gc_arena, int *is_delegate)
static void send_response(h2o_mruby_generator_t *generator, mrb_int status, mrb_value resp, int *is_delegate)
{
mrb_state *mrb = generator->ctx->mrb;
mrb_value body;
Expand All @@ -614,7 +614,6 @@ static void send_response(h2o_mruby_generator_t *generator, mrb_int status, mrb_
if (generator->req->res.status == STATUS_FALLTHRU) {
assert(is_delegate != NULL);
*is_delegate = 1;
mrb_gc_arena_restore(mrb, gc_arena);
return;
}

Expand Down Expand Up @@ -652,11 +651,8 @@ static void send_response(h2o_mruby_generator_t *generator, mrb_int status, mrb_
if (!mrb_nil_p(body)) {
h2o_start_response(generator->req, &generator->super);
mrb_value receiver = h2o_mruby_send_chunked_init(generator, body);
if (mrb_nil_p(receiver)) {
mrb_gc_arena_restore(mrb, gc_arena);
} else {
h2o_mruby_run_fiber(generator, receiver, body, gc_arena, 0);
}
if (!mrb_nil_p(receiver))
h2o_mruby_run_fiber(generator, receiver, body, 0);
return;
}

Expand All @@ -675,11 +671,10 @@ static void send_response(h2o_mruby_generator_t *generator, mrb_int status, mrb_
GotException:
report_exception(generator->req, mrb);
SendInternalError:
mrb_gc_arena_restore(mrb, gc_arena);
h2o_send_error(generator->req, 500, "Internal Server Error", "Internal Server Error", 0);
}

void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, mrb_value input, int gc_arena, int *is_delegate)
void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, mrb_value input, int *is_delegate)
{
mrb_state *mrb = generator->ctx->mrb;
mrb_value output;
Expand Down Expand Up @@ -735,7 +730,6 @@ void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, m
}
switch (next_action) {
case H2O_MRUBY_CALLBACK_NEXT_ACTION_STOP:
mrb_gc_arena_restore(mrb, gc_arena);
return;
case H2O_MRUBY_CALLBACK_NEXT_ACTION_ASYNC:
goto Async;
Expand All @@ -748,7 +742,6 @@ void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, m
/* if no special actions were necessary, then the output is a rack response */
break;
Next:
mrb_gc_arena_restore(mrb, gc_arena);
mrb_gc_protect(mrb, receiver);
mrb_gc_protect(mrb, input);
}
Expand All @@ -761,15 +754,13 @@ void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, m
}

/* send the response (unless req is already closed) */
if (generator->req == NULL) {
mrb_gc_arena_restore(mrb, gc_arena);
if (generator->req == NULL)
return;
}
if (generator->req->_generator != NULL) {
mrb->exc = mrb_obj_ptr(mrb_exc_new_str_lit(mrb, E_RUNTIME_ERROR, "unexpectedly received a rack response"));
goto GotException;
}
send_response(generator, status, output, gc_arena, is_delegate);
send_response(generator, status, output, is_delegate);
return;

GotException:
Expand All @@ -782,12 +773,10 @@ void h2o_mruby_run_fiber(h2o_mruby_generator_t *generator, mrb_value receiver, m
h2o_mruby_send_chunked_close(generator);
}
}
mrb_gc_arena_restore(mrb, gc_arena);
return;

Async:
h2o_mruby_current_generator = NULL;
mrb_gc_arena_restore(mrb, gc_arena);
if (!mrb_obj_eq(mrb, generator->ctx->proc, receiver))
mrb_gc_register(mrb, receiver);
return;
Expand Down
10 changes: 7 additions & 3 deletions lib/handler/mruby/http_request.c
View file
Expand Up @@ -102,7 +102,8 @@ static void on_dispose(void *_ctx)
if (!mrb_nil_p(ctx->receiver)) {
mrb_state *mrb = ctx->generator->ctx->mrb;
int gc_arena = mrb_gc_arena_save(mrb);
h2o_mruby_run_fiber(ctx->generator, detach_receiver(ctx), create_downstream_closed_exception(mrb), gc_arena, NULL);
h2o_mruby_run_fiber(ctx->generator, detach_receiver(ctx), create_downstream_closed_exception(mrb), NULL);
mrb_gc_arena_restore(mrb, gc_arena);
}
}

Expand Down Expand Up @@ -153,8 +154,10 @@ static void post_response(struct st_h2o_mruby_http_request_context_t *ctx, int s
}
} else {
/* send response to the waiting receiver */
h2o_mruby_run_fiber(ctx->generator, detach_receiver(ctx), resp, gc_arena, NULL);
h2o_mruby_run_fiber(ctx->generator, detach_receiver(ctx), resp, NULL);
}

mrb_gc_arena_restore(mrb, gc_arena);
}

static void post_error(struct st_h2o_mruby_http_request_context_t *ctx, const char *errstr)
Expand Down Expand Up @@ -215,7 +218,8 @@ static int on_body(h2o_http1client_t *client, const char *errstr)
} else if (!mrb_nil_p(ctx->receiver)) {
int gc_arena = mrb_gc_arena_save(ctx->generator->ctx->mrb);
mrb_value chunk = build_chunk(ctx);
h2o_mruby_run_fiber(ctx->generator, detach_receiver(ctx), chunk, gc_arena, NULL);
h2o_mruby_run_fiber(ctx->generator, detach_receiver(ctx), chunk, NULL);
mrb_gc_arena_restore(ctx->generator->ctx->mrb, gc_arena);
}
}
return 0;
Expand Down

0 comments on commit e49fdd7

Please sign in to comment.