C ext: GC claiming objects early

ruby-talk
1

Hi,
I'm implementing Ruby bindings for a C library and I'm encountering some
problems related to the garbage collector, my objects are claimed too
early.

Here's the ruby code:

o.some_meth.notifier do |r|
   do_stuff_with(r)
end

In the C library, functions are processed asynchronously, so everything
is managed with intermediate Result objects.

"some_meth" will instantiate a new "Result" object. Using this Result
object, the caller can wait for the function to succeed, he can register
callbacks etc.

The Result objects notifier method registers a callback - its called
when "some_meth" finishes, which can take some time :wink:
This callback might be called more than once.

In my test script, I made sure the handler is called continously.
By calling GC.start in the handler, I found out that the GC is claiming
the Result object early, since there are no objects that are marking it.

The only object that's referencing the Result object is the block at
this time, and I guess it doesn't actually mark the object :slight_smile:

I know I can work around this by using rb_global_variable() to tell the
GC not to claim the object, but this kinda sucks since the object will
never be freed :frowning:

So, my question is: how do I prevent the Result object from being
claimed by the GC without using rb_global_variable's?

What are common workarounds for these types of problems?
Is there any online documentation about the topic (besides README.EXT
and the Pickaxe's section on Extending Ruby)?

Thanks in advance.

···

--
Regards,
Tilman

2

If this object is a VALUE variable in your C code, you should declare it this
way:

    volatile VALUE result = Qnil;

Adding volatile allows the GC to see the object on the C stack. This means
that so long as your C code holds it in that variable, it will never go away.

  Sean O'Dell

···

On Thursday 29 July 2004 13:11, Tilman Sauerbeck wrote:

I know I can work around this by using rb_global_variable() to tell the
GC not to claim the object, but this kinda sucks since the object will
never be freed :frowning:

So, my question is: how do I prevent the Result object from being
claimed by the GC without using rb_global_variable's?

3

"some_meth" will instantiate a new "Result" object. Using this Result
object, the caller can wait for the function to succeed, he can register
callbacks etc.

Well it's best if you post the *real* C code : it's easy to understand
than this strange language, called english.

Is there any online documentation about the topic (besides README.EXT
and the Pickaxe's section on Extending Ruby)?

eval.c and gc.c :slight_smile:

Guy Decoux

4

Sean O'Dell wrote:

···

On Thursday 29 July 2004 13:11, Tilman Sauerbeck wrote:

I know I can work around this by using rb_global_variable() to tell the
GC not to claim the object, but this kinda sucks since the object will
never be freed :frowning:

So, my question is: how do I prevent the Result object from being
claimed by the GC without using rb_global_variable's?

If this object is a VALUE variable in your C code, you should declare it
this way:

    volatile VALUE result = Qnil;

Adding volatile allows the GC to see the object on the C stack. This
means that so long as your C code holds it in that variable, it will never
go away.

What about using rb_gc_register_address() to ensure the result object isn't
gc'd?

VALUE Result;
....

rb_gc_register_address(&Result);

-- Richard

5

Sean O'Dell <sean@celsoft.com> [2004-07-30 15:42]:

···

On Thursday 29 July 2004 13:11, Tilman Sauerbeck wrote:
>
> I know I can work around this by using rb_global_variable() to tell the
> GC not to claim the object, but this kinda sucks since the object will
> never be freed :frowning:
>
> So, my question is: how do I prevent the Result object from being
> claimed by the GC without using rb_global_variable's?

If this object is a VALUE variable in your C code, you should declare it this
way:

    volatile VALUE result = Qnil;

Adding volatile allows the GC to see the object on the C stack. This means
that so long as your C code holds it in that variable, it will never go away.

Okay, but that wouldn't really be different from using
rb_global_variable() and rb_gc_unregister_address(), right?

I just noticed my last question was badly worded :confused:
I _do_ want the GC to claim the object eventually, but not if the
callback might still be called.

--
Regards,
Tilman

6

ts <decoux@moulon.inra.fr> [2004-07-30 15:43]:

> "some_meth" will instantiate a new "Result" object. Using this Result
> object, the caller can wait for the function to succeed, he can register
> callbacks etc.

Well it's best if you post the *real* C code : it's easy to understand
than this strange language, called english.

Heh :slight_smile:

Here's the code for the method "some_meth", let's just call it
"playback_playtime" now ;D

static VALUE c_playback_playtime (VALUE self)
{
  xmmsc_result_t *res;

  /* this just wraps Data_Get_Struct */
  GET_OBJ (self, xmmsc_connection_t *, xmms);

  res = xmmsc_playback_playtime (*xmms);

  /* now create a new Result object from the C struct */
  return TO_XMMS_CLIENT_RESULT (self, res);
}

VALUE TO_XMMS_CLIENT_RESULT (VALUE parent, xmmsc_result_t *res)
{
  VALUE self;
  RbResult *rbres = NULL;

  if (!res)
    return Qnil;

  self = Data_Make_Struct (cResult, RbResult, c_mark, c_free, rbres);
  rbres->real = res;
  rbres->parent = parent;
  rbres->callback = Qnil;

  rb_obj_call_init (rbres->self, 0, NULL);

  return self;
}

'parent' is the Ruby object that the object marks in c_mark

This is the "notifier" method of the Result object and the callback used
to call the given block:

static void on_signal (xmmsc_result_t *res2, void *data)
{
  /* call the block and create a new Result object from res2, too */
  rb_funcall ((VALUE) data, rb_intern ("call"), 1,
              TO_XMMS_CLIENT_RESULT (Qnil, res2));
}

static VALUE c_notifier_set (VALUE self)
{
  GET_OBJ (self, RbResult, res);

  if (!rb_block_given_p ())
    return Qnil;

  res->callback = rb_block_proc ();
  xmmsc_result_notifier_set (res->real, on_signal, (void *) res->callback);

  return Qnil;
}

It's pretty straight-forward and the comment should explain it all :slight_smile:

Thanks for any hints.

···

--
Regards,
Tilman

7

Sean O'Dell <sean@celsoft.com> [2004-07-30 15:42]:
> > I know I can work around this by using rb_global_variable() to tell the
> > GC not to claim the object, but this kinda sucks since the object will
> > never be freed :frowning:
> >
> > So, my question is: how do I prevent the Result object from being
> > claimed by the GC without using rb_global_variable's?
>
> If this object is a VALUE variable in your C code, you should declare it
> this way:
>
> volatile VALUE result = Qnil;
>
> Adding volatile allows the GC to see the object on the C stack. This
> means that so long as your C code holds it in that variable, it will
> never go away.

Okay, but that wouldn't really be different from using
rb_global_variable() and rb_gc_unregister_address(), right?

It's completely different; it's for VALUEs on the stack. By declaring it
volatile, you force the VALUE on the C stack, where the GC can see it. The
variable can be local to a C function and if the GC is called, it will see it
and not collect it.

I just noticed my last question was badly worded :confused:
I _do_ want the GC to claim the object eventually, but not if the
callback might still be called.

I haven't seen your C code, so I'm not sure what is happening, but if you are
maintaining the VALUE as a global and returning control to Ruby, then trying
to use the global in a callback, you should register the VALUE with
rb_global_variable or rb_gc_register_address to keep the GC from collecting
it.

  Sean O'Dell

···

On Friday 30 July 2004 06:56, Tilman Sauerbeck wrote:

> On Thursday 29 July 2004 13:11, Tilman Sauerbeck wrote:

8

Show me a couple more things. Show me the Ruby code you call, and tell me
what object is getting collected and where it happens. Your original Ruby
code example was pseudocode and I can't tell what maps to what.

  Sean O'Dell

···

On Friday 30 July 2004 07:06, Tilman Sauerbeck wrote:

ts <decoux@moulon.inra.fr> [2004-07-30 15:43]:

>
> > "some_meth" will instantiate a new "Result" object. Using this Result
> > object, the caller can wait for the function to succeed, he can
> register T> callbacks etc.
>
> Well it's best if you post the *real* C code : it's easy to understand
> than this strange language, called english.

Heh :slight_smile:

Here's the code for the method "some_meth", let's just call it
"playback_playtime" now ;D

static VALUE c_playback_playtime (VALUE self)
{
  xmmsc_result_t *res;

  /* this just wraps Data_Get_Struct */
  GET_OBJ (self, xmmsc_connection_t *, xmms);

  res = xmmsc_playback_playtime (*xmms);

  /* now create a new Result object from the C struct */
  return TO_XMMS_CLIENT_RESULT (self, res);
}

VALUE TO_XMMS_CLIENT_RESULT (VALUE parent, xmmsc_result_t *res)
{
  VALUE self;
  RbResult *rbres = NULL;

  if (!res)
    return Qnil;

  self = Data_Make_Struct (cResult, RbResult, c_mark, c_free, rbres);
  rbres->real = res;
  rbres->parent = parent;
  rbres->callback = Qnil;

  rb_obj_call_init (rbres->self, 0, NULL);

  return self;
}

'parent' is the Ruby object that the object marks in c_mark

This is the "notifier" method of the Result object and the callback used
to call the given block:

static void on_signal (xmmsc_result_t *res2, void *data)
{
  /* call the block and create a new Result object from res2, too */
  rb_funcall ((VALUE) data, rb_intern ("call"), 1,
              TO_XMMS_CLIENT_RESULT (Qnil, res2));
}

static VALUE c_notifier_set (VALUE self)
{
  GET_OBJ (self, RbResult, res);

  if (!rb_block_given_p ())
    return Qnil;

  res->callback = rb_block_proc ();
  xmmsc_result_notifier_set (res->real, on_signal, (void *) res->callback);

  return Qnil;
}

It's pretty straight-forward and the comment should explain it all :slight_smile:

9

  rb_obj_call_init (rbres->self, 0, NULL);

this is, no ?

        rb_obj_call_init(self, 0, NULL);

static VALUE c_notifier_set (VALUE self)

Well, you attach a notifier to an object but with the construct

   o.some_meth.notifier do |r|
      do_stuff_with(r)
   end

this object is never stored in a variable, this is why ruby remove it

Normally you must write it

   res = o.some_method
   res.notifier do |r|
      do_stuff_with(r)
   end

and it will work, until ruby call the GC for `res'

If you want to make it work like it is actually, you must store the result
of `o.some_method' in an Array defined in `o' when #notifier is called,
and mark this array. It will be marked when ruby will mark `o'

Now the problem is here

I _do_ want the GC to claim the object eventually, but not if the
callback might still be called.

How do you know that the callback might still be called ?

Because if you use a method to release a notifier, then you just need an
Array (defined with rb_global_variable()) which store the object when
#notifier is called and remove it when you release the notifier

Guy Decoux

10

ts <decoux@moulon.inra.fr> [2004-07-30 15:43]:

> "some_meth" will instantiate a new "Result" object. Using this Result
> object, the caller can wait for the function to succeed, he can register
> callbacks etc.

Well it's best if you post the *real* C code : it's easy to understand
than this strange language, called english.

Heh :slight_smile:

Here's the code for the method "some_meth", let's just call it
"playback_playtime" now ;D

static VALUE c_playback_playtime (VALUE self)
{
  xmmsc_result_t *res;

  /* this just wraps Data_Get_Struct */
  GET_OBJ (self, xmmsc_connection_t *, xmms);

  res = xmmsc_playback_playtime (*xmms);

  /* now create a new Result object from the C struct */
  return TO_XMMS_CLIENT_RESULT (self, res);
}

VALUE TO_XMMS_CLIENT_RESULT (VALUE parent, xmmsc_result_t *res)
{
  VALUE self;
  RbResult *rbres = NULL;

  if (!res)
    return Qnil;

  self = Data_Make_Struct (cResult, RbResult, c_mark, c_free, rbres);
  rbres->real = res;
  rbres->parent = parent;
  rbres->callback = Qnil;

You should show us c_mark and c_free
In extreme situations the above code it dangerous, here is a safer version:
<code>
  VALUE self;
  RbResult *rbres;

  if (!res)
    return Qnil;

  rbres = ALLOC(RbRecsult);
  rbres->real = res;
  rbres->parent = parent;
  rbres->callback = Qnil;
  self = Data_Wrap_Struct (cResult, c_mark, c_free, rbres);
</code>

  rb_obj_call_init (rbres->self, 0, NULL);

  return self;

Where did rbres->self come from? Doesn't look like you initialized it. You need to pass a valid Ruby object to rb_obj_call_init(), perhaps you intended:
<code>
  return rb_obj_call_init(self, 0, 0); /* returns self */
</code>

}

'parent' is the Ruby object that the object marks in c_mark

This is the "notifier" method of the Result object and the callback used
to call the given block:

static void on_signal (xmmsc_result_t *res2, void *data)
{
  /* call the block and create a new Result object from res2, too */
  rb_funcall ((VALUE) data, rb_intern ("call"), 1,
              TO_XMMS_CLIENT_RESULT (Qnil, res2));
}

static VALUE c_notifier_set (VALUE self)
{
  GET_OBJ (self, RbResult, res);

  if (!rb_block_given_p ())
    return Qnil;

  res->callback = rb_block_proc ();
  xmmsc_result_notifier_set (res->real, on_signal, (void *) res->callback);

This could be your problem. We encountered a similar problem last night with the Rendezvous Ruby wrappers.
rb_block_proc() creates a ruby object.
your registering on_signal as your event handler, and it looks like what ever library you are using (you should supply more code here) will the pass the void* to the call back for you. You need to show c_mark() so it is clear that your marking everything.
-Charlie

···

On Jul 30, 2004, at 7:06 AM, Tilman Sauerbeck wrote:

  return Qnil;
}

It's pretty straight-forward and the comment should explain it all :slight_smile:

Thanks for any hints.

--
Regards,
Tilman

11

ts <decoux@moulon.inra.fr> [2004-07-30 17:20]:

> rb_obj_call_init (rbres->self, 0, NULL);

this is, no ?

        rb_obj_call_init(self, 0, NULL);

Yes, that's it. rbres->self came from an older version I experimented
with.

> static VALUE c_notifier_set (VALUE self)

Well, you attach a notifier to an object but with the construct

   o.some_meth.notifier do |r|
      do_stuff_with(r)
   end

this object is never stored in a variable, this is why ruby remove it

Normally you must write it

   res = o.some_method
   res.notifier do |r|
      do_stuff_with(r)
   end

and it will work, until ruby call the GC for `res'

If you want to make it work like it is actually, you must store the result
of `o.some_method' in an Array defined in `o' when #notifier is called,
and mark this array. It will be marked when ruby will mark `o'

Yeah. Although if I make the parent object (the XmmsClient, 'o') know
about (and mark) the Result objects, I've got a circular reference,
since the result objects mark their parent, which is 'o'.
I do it that way to make sure all result objects are claimed before the
XmmsClient object.

Now the problem is here

> I _do_ want the GC to claim the object eventually, but not if the
> callback might still be called.

How do you know that the callback might still be called ?

No idea :slight_smile:
I hoped Ruby would make the block object mark the result object, since
it allocated the block.

This would actually solve all of my problems at this point, I think.

···

--
Regards,
Tilman

12

Sean O'Dell <sean@celsoft.com> [2004-07-30 16:42]:

> [C code]
> It's pretty straight-forward and the comment should explain it all :slight_smile:

Show me a couple more things. Show me the Ruby code you call, and tell me
what object is getting collected and where it happens. Your original Ruby
code example was pseudocode and I can't tell what maps to what.

Anyway, here's the script:

class Basic
  def initialize
    @xc = XmmsClient::XmmsClient::new
    @xc.connect
    @xc.setup_with_ecore # attach to the main loop, see below

    @xc.playlist_current_id.notifier do |r|
      puts "now playing #{r.uint}" # gets the ID from a result
      r.restart # call the block the next time the event gets
                # fired
      GC.start
    end

    @xc.playback_playtime.notifier do |r|
      r.restart
      GC.start
    end
  end
end

x = Basic::new
Ecore::main_loop_begin

The 2nd block is continously prints the current playtime of a track.
The first block prints the ID of the current track on track changes.

The segfault occurs when the first block is executed for the 2nd time
(at this point, the 2nd block has been executed several times already).

Here's the valgrind log:

CREATED result: 0x1bc194e0 /* result for the playlist_current_id */
CREATED result: 0x1bc1b178 /* result for playback_playtime */
now playing: 5
FREED result: 0x1bc1b178
FREED result: 0x1bc194e0
==2096== Invalid read of size 4
==2096== at 0x1B9A6D94: st_lookup (st.c:258)
==2096== by 0x1B93510F: search_method (eval.c:378)
==2096== by 0x1B935185: rb_get_method_body (eval.c:399)
==2096== by 0x1B94167F: rb_call (eval.c:5263)
==2096== by 0x1B941A05: rb_funcall (eval.c:5352)
==2096== by 0x1B909BE6: on_signal (rb_result.c:107)

So both objects are freed and thus, res->callback is claimed too.

···

On Friday 30 July 2004 07:06, Tilman Sauerbeck wrote:

--
Regards,
Tilman

13

Charles Mills <cmills@freeshell.org> [2004-07-30 19:31]:

>ts <decoux@moulon.inra.fr> [2004-07-30 15:43]:
>>>>>>>"T" == Tilman Sauerbeck <tilman@code-monkey.de> writes:
>>
>>> "some_meth" will instantiate a new "Result" object. Using this
>>Result
>>> object, the caller can wait for the function to succeed, he can
>>register
>>> callbacks etc.
>>
>> Well it's best if you post the *real* C code : it's easy to
>>understand
>> than this strange language, called english.
>
>Heh :slight_smile:
>
>[C code]
You should show us c_mark and c_free

static void c_mark (RbResult *res)
{
  if (!NIL_P (res->parent))
    rb_gc_mark (res->parent);

  if (!NIL_P (res->callback))
    rb_gc_mark (res->callback);
}

static void c_free (RbResult *res)
{
  if (res->real) {
    xmmsc_result_unref (res->real);
    res->real = NULL;
  }

  res->parent = Qnil;

  free (res);
}

In extreme situations the above code it dangerous, here is a safer
version:
<code>
  VALUE self;
  RbResult *rbres;

  if (!res)
    return Qnil;

  rbres = ALLOC(RbRecsult);
  rbres->real = res;
  rbres->parent = parent;
  rbres->callback = Qnil;
  self = Data_Wrap_Struct (cResult, c_mark, c_free, rbres);
</code>

You mean the memset() call of Data_Make_Struct is dangerous?
I kind of doubt memset is causing problems here, but I'll try it anyway
:slight_smile:

> rb_obj_call_init (rbres->self, 0, NULL);
>
> return self;
Where did rbres->self come from? Doesn't look like you initialized it.
You need to pass a valid Ruby object to rb_obj_call_init(), perhaps
you intended:
<code>
  return rb_obj_call_init(self, 0, 0); /* returns self */
</code>

Yeah, it was a typo which didn't occur in my original code *hides*

>
>'parent' is the Ruby object that the object marks in c_mark
>
>This is the "notifier" method of the Result object and the callback
>used
>to call the given block:
>
>static void on_signal (xmmsc_result_t *res2, void *data)
>{
> /* call the block and create a new Result object from res2, too */
> rb_funcall ((VALUE) data, rb_intern ("call"), 1,
> TO_XMMS_CLIENT_RESULT (Qnil, res2));
>}
>
>static VALUE c_notifier_set (VALUE self)
>{
> GET_OBJ (self, RbResult, res);
>
> if (!rb_block_given_p ())
> return Qnil;
>
> res->callback = rb_block_proc ();
> xmmsc_result_notifier_set (res->real, on_signal, (void *)
>res->callback);
This could be your problem. We encountered a similar problem last night
with the Rendezvous Ruby wrappers.
rb_block_proc() creates a ruby object.
your registering on_signal as your event handler, and it looks like
what ever library you are using (you should supply more code here) will
the pass the void* to the call back for you. You need to show c_mark()
so it is clear that your marking everything.

Yes, the library does pass res->callback as the data pointer to
on_signal. You are right, I should have commented on it, but it's such a
common technique in C I thought I wouldn't have to explain it :stuck_out_tongue:

···

On Jul 30, 2004, at 7:06 AM, Tilman Sauerbeck wrote:

--
Regards,
Tilman

14

Sorry, hopefully Guy can help you. I can't match up any of those calls to the
C code you provided. Maybe it's my eyes, but I see only vague similarities
like the letters XMMS appearing in both places, but I don't see Ruby code
that matches up with your C code.

  Sean O'Dell

···

On Friday 30 July 2004 11:29, Tilman Sauerbeck wrote:

Sean O'Dell <sean@celsoft.com> [2004-07-30 16:42]:
> On Friday 30 July 2004 07:06, Tilman Sauerbeck wrote:
> > [C code]
> > It's pretty straight-forward and the comment should explain it all :slight_smile:
>
> Show me a couple more things. Show me the Ruby code you call, and tell
> me what object is getting collected and where it happens. Your original
> Ruby code example was pseudocode and I can't tell what maps to what.

Anyway, here's the script:

class Basic
  def initialize
    @xc = XmmsClient::XmmsClient::new
    @xc.connect
    @xc.setup_with_ecore # attach to the main loop, see below

    @xc.playlist_current_id.notifier do |r|
      puts "now playing #{r.uint}" # gets the ID from a result
      r.restart # call the block the next time the event gets
                # fired
      GC.start
    end

    @xc.playback_playtime.notifier do |r|
      r.restart
      GC.start
    end
  end
end

15

Yeah. Although if I make the parent object (the XmmsClient, 'o') know
about (and mark) the Result objects, I've got a circular reference,
since the result objects mark their parent, which is 'o'.

Circular reference is not a problem because the GC is a mark-sweep GC.

bdb use circular reference and it work fine.

For example (this is cats.rb)

   def tt
      bdb = BDB::Btree.open "tmp/aa", nil, "w", "marshal" => true
      aux = BDB::Btree.open "tmp/bb", nil, "w",
         "set_flags" => BDB::DUPSORT, "marshal" => true
      bdb.associate(aux) { |aux1, key, value| value.life }
      nil
   end

   tt
   GC.start

`bdb' will mark `aux' and `aux' will mark `bdb'

When the GC run, it will not mark `bdb' because it has no reference to it,
and it will not mark `aux' for the same reason. At the sweep phase, `bdb'
and `aux' are removed (closed in this case)

Now with

   def tt
      bdb = BDB::Btree.open "tmp/aa", nil, "w", "marshal" => true
      aux = BDB::Btree.open "tmp/bb", nil, "w",
         "set_flags" => BDB::DUPSORT, "marshal" => true
      bdb.associate(aux) { |aux1, key, value| value.life }
      bdb
   end

   a = tt
   GC.start

the GC will mark `a' (i.e. `bdb') which mark `aux' (same if it return
`aux' rather than `bdb')

Guy Decoux

16

ts <decoux@moulon.inra.fr> [2004-07-31 13:06]:

> Yeah. Although if I make the parent object (the XmmsClient, 'o') know
> about (and mark) the Result objects, I've got a circular reference,
> since the result objects mark their parent, which is 'o'.

Circular reference is not a problem because the GC is a mark-sweep GC.

[code]
`bdb' will mark `aux' and `aux' will mark `bdb'

When the GC run, it will not mark `bdb' because it has no reference to it,
and it will not mark `aux' for the same reason. At the sweep phase, `bdb'
and `aux' are removed (closed in this case)

Ah, so the GC might omit the call to the mark callback before the free
callback is called? Until now I thought the mark function was always
executed.

So far my objects only marked the objects they depend upon, their
parents so to say. I'll change this so they mark all the objects they
get in touch with =)

Thank you :slight_smile:

···

--
Regards,
Tilman

17

Tilman Sauerbeck wrote:

So far my objects only marked the objects they depend upon, their
parents so to say. I'll change this so they mark all the objects they
get in touch with =)

Thank you :slight_smile:

The Pickaxe says that the purpose of the mark routine is "to mark any
references from your structure to other structures...If your structure
references other Ruby objects, then your mark function needs to identify
those objects using rb_gc_mark(value)."