8:04 a.m.
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.
8:04 a.m.
New subject: [PATCH 1/3] ruby: Append newline character after printing exception in callback.
From: "Richard W.M. Jones" <rjones(a)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
8:04 a.m.
New subject: [PATCH 2/3] ruby: Use a regular C array to pass the arguments through rb_rescue.
From: "Richard W.M. Jones" <rjones(a)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
8:05 a.m.
New subject: [PATCH 3/3] ruby: Check Ruby callback exists before we call it (RHBZ#733297).
From: "Richard W.M. Jones" <rjones(a)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
8:07 a.m.
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
5217
days inactive
5218
days old
4 comments
2 participants
participants (2)
-
Chris Lalancette -
Richard W.M. Jones