Libguestfs - Aug 2011 - [PATCH 0/3] ruby: Fix event handler failure

I won't pretend I really understand what's going on here.  I've
CC'd
this message to Chris since he might have a better idea.

  https://bugzilla.redhat.com/show_bug.cgi?id=733297

In this bug, it appears that the event log callback goes out of scope
and is garbage collected.  (This is despite the fact we registered it
as a global root).  When we invoke the callback later, rb_funcall
raises this exception:

  wrong argument type Proc (expected Data)

I checked the type of the callback, and indeed it changes from T_DATA
(normal) to T_NONE (raises this exception).

My workaround simply checks that the type != T_NONE before calling.

Rich.

From: "Richard W.M. Jones" <rjones at redhat.com>

---
 generator/generator_ruby.ml |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/generator/generator_ruby.ml b/generator/generator_ruby.ml
index a21334a..4e12000 100644
--- a/generator/generator_ruby.ml
+++ b/generator/generator_ruby.ml
@@ -264,7 +264,7 @@ ruby_event_callback_handle_exception (VALUE not_used, VALUE
exn)
   /* Callbacks aren't supposed to throw exceptions.  The best we
    * can do is to print the error.
    */
-  fprintf (stderr, \"libguestfs: exception in callback: %%s\",
+  fprintf (stderr, \"libguestfs: exception in callback: %%s\\n\",
            StringValueCStr (exn));
 
   return Qnil;
-- 
1.7.6

From: "Richard W.M. Jones" <rjones at redhat.com>

---
 generator/generator_ruby.ml |   35 +++++++++++++++++------------------
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/generator/generator_ruby.ml b/generator/generator_ruby.ml
index 4e12000..eee6b7e 100644
--- a/generator/generator_ruby.ml
+++ b/generator/generator_ruby.ml
@@ -214,7 +214,8 @@ ruby_event_callback_wrapper (guestfs_h *g,
                              const uint64_t *array, size_t array_len)
 {
   size_t i;
-  VALUE eventv, event_handlev, bufv, arrayv, argv;
+  VALUE eventv, event_handlev, bufv, arrayv;
+  VALUE argv[5];
 
   eventv = ULL2NUM (event);
   event_handlev = INT2NUM (event_handle);
@@ -225,32 +226,30 @@ ruby_event_callback_wrapper (guestfs_h *g,
   for (i = 0; i < array_len; ++i)
     rb_ary_push (arrayv, ULL2NUM (array[i]));
 
-  /* Wrap up the arguments in an array which will be unpacked
-   * and passed as multiple arguments.  This is a crap limitation
-   * of rb_rescue.
+  /* This is a crap limitation of rb_rescue.
    * http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/~poffice/mail/ruby-talk/65698
    */
-  argv = rb_ary_new2 (5);
-  rb_ary_store (argv, 0, * (VALUE *) data /* function */);
-  rb_ary_store (argv, 1, eventv);
-  rb_ary_store (argv, 2, event_handlev);
-  rb_ary_store (argv, 3, bufv);
-  rb_ary_store (argv, 4, arrayv);
-
-  rb_rescue (ruby_event_callback_wrapper_wrapper, argv,
+  argv[0] = * (VALUE *) data; /* function */
+  argv[1] = eventv;
+  argv[2] = event_handlev;
+  argv[3] = bufv;
+  argv[4] = arrayv;
+
+  rb_rescue (ruby_event_callback_wrapper_wrapper, (VALUE) argv,
              ruby_event_callback_handle_exception, Qnil);
 }
 
 static VALUE
-ruby_event_callback_wrapper_wrapper (VALUE argv)
+ruby_event_callback_wrapper_wrapper (VALUE argvv)
 {
+  VALUE *argv = (VALUE *) argvv;
   VALUE fn, eventv, event_handlev, bufv, arrayv;
 
-  fn = rb_ary_entry (argv, 0);
-  eventv = rb_ary_entry (argv, 1);
-  event_handlev = rb_ary_entry (argv, 2);
-  bufv = rb_ary_entry (argv, 3);
-  arrayv = rb_ary_entry (argv, 4);
+  fn = argv[0];
+  eventv = argv[1];
+  event_handlev = argv[2];
+  bufv = argv[3];
+  arrayv = argv[4];
 
   rb_funcall (fn, rb_intern (\"call\"), 4,
               eventv, event_handlev, bufv, arrayv);
-- 
1.7.6

From: "Richard W.M. Jones" <rjones at redhat.com>

---
 generator/generator_ruby.ml |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/generator/generator_ruby.ml b/generator/generator_ruby.ml
index eee6b7e..38121b5 100644
--- a/generator/generator_ruby.ml
+++ b/generator/generator_ruby.ml
@@ -246,13 +246,21 @@ ruby_event_callback_wrapper_wrapper (VALUE argvv)
   VALUE fn, eventv, event_handlev, bufv, arrayv;
 
   fn = argv[0];
-  eventv = argv[1];
-  event_handlev = argv[2];
-  bufv = argv[3];
-  arrayv = argv[4];
 
-  rb_funcall (fn, rb_intern (\"call\"), 4,
-              eventv, event_handlev, bufv, arrayv);
+  /* Check the Ruby callback still exists.  For reasons which are not
+   * fully understood, even though we registered this as a global root,
+   * it is still possible for the callback to go away (fn value remains
+   * but its type changes from T_DATA to T_NONE).  (RHBZ#733297)
+   */
+  if (rb_type (fn) != T_NONE) {
+    eventv = argv[1];
+    event_handlev = argv[2];
+    bufv = argv[3];
+    arrayv = argv[4];
+
+    rb_funcall (fn, rb_intern (\"call\"), 4,
+                eventv, event_handlev, bufv, arrayv);
+  }
 
   return Qnil;
 }
-- 
1.7.6

On 08/25/11 - 02:04:57PM, Richard W.M. Jones wrote:
> I won't pretend I really understand what's going on here.  I've
CC'd
> this message to Chris since he might have a better idea.
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=733297
> 
> In this bug, it appears that the event log callback goes out of scope
> and is garbage collected.  (This is despite the fact we registered it
> as a global root).  When we invoke the callback later, rb_funcall
> raises this exception:
> 
>   wrong argument type Proc (expected Data)
> 
> I checked the type of the callback, and indeed it changes from T_DATA
> (normal) to T_NONE (raises this exception).
> 
> My workaround simply checks that the type != T_NONE before calling.
Nothing jumps to mind here Rich.  I took a quick look at the code in libguestfs
that is doing this, and it seems sane, so I don't have a good explanation
for
you.  If I find some time this afternoon I'll take a deeper look and see if
I
come up with any ideas.

-- 
Chris Lalancette