mattn / mruby-uv

interface to libuv for mruby(experimental)
http://mattn.kaoriya.net/
92 stars 25 forks source link

memory leak - connections never freed, even with GC #21

Closed client9 closed 10 years ago

client9 commented 10 years ago

Hi again :-)

Its a mruby-uv bug, but its easy to see with your mruby-http https://github.com/mattn/mruby-http/blob/master/example/server.rb

A new context is being created in accept, but its never free, even with GC.

in function mrb_uv_close

   uv_close(&context->any.handle, close_cb);
+  context->mrb = NULL;
   return mrb_nil_value();

this allows mrb_uv_gc to correctly free the context

Thoughts? Not sure my fix is correct.

mattn commented 10 years ago

uv_close() call close_cb in async. And close_cb use context->mrb in later. So we can't set NULL in this part. mrubu-uv free them at uv_context_free.

mattn commented 10 years ago

See mrb_uv_gc

client9 commented 10 years ago

Ahh yes the close_cb

The gc is line 98

      if (!context || context->mrb == NULL) {
    mrb_funcall(mrb, uv_gc_table, "delete_at", 1, mrb_fixnum_value(i));

is the condition to free memory.

So...

it needs to be called after _uv_close_cb is run.

Any suggestions here?

nickg

mattn commented 10 years ago

it needs to be called after uvclose_cb is run.

Yes.

client9 commented 10 years ago

idea -- https://github.com/client9/mruby-uv/commit/364169f4e5e27266d81cc1207ac09d009f3cd142

  1. always call _uv_close_cb (before it only called if there was mruby user callback)
  2. if user callback exists, just exec the function, don't yield
  3. delete reference
  4. (gc now works correctly)

On Feb 21, 2014, at 11:15, mattn notifications@github.com wrote:

it needs to be called after uvclose_cb is run.

Yes.

— Reply to this email directly or view it on GitHub.

mattn commented 10 years ago

Ah, nice.

mattn commented 10 years ago

Could you please send me a pull-req?