ruby-changes:62847
From: Yusuke <ko1@a...>
Date: Sun, 6 Sep 2020 13:58:04 +0900 (JST)
Subject: [ruby-changes:62847] 369cfabd59 (master): Make it possible to dump and load an exception object
https://git.ruby-lang.org/ruby.git/commit/?id=369cfabd59 From 369cfabd5936ccb522f8e95e0f9cc65b59ea4039 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh <mame@r...> Date: Sat, 5 Sep 2020 21:18:45 +0900 Subject: Make it possible to dump and load an exception object A backtrace object in an exception had never supported marshalling correctly: `Marshal.load(Marshal.dump(exc)).backtrace_locations` dumped core. An Exception object has two hidden instance varibles for backtrace data: one is "bt", which has an Array of Strings, and the other is "bt_locations", which has an Array of Thread::Backtrace::Locations. However, Exception's dump outputs data so that the two variables are the same Array of Strings. Thus, "bt_locations" had a wrong-type object. For the compatibility, it is difficult to change the dump format. This changeset fixes the issue by ignoring data for "bt_locations" at the loading phase if "bt_locations" refers to the same object as "bt". Future work: Exception's dump should output "bt_locations" appropriately. https://bugs.ruby-lang.org/issues/17150 diff --git a/error.c b/error.c index 70f24a0..61fa86b 100644 --- a/error.c +++ b/error.c @@ -2599,10 +2599,56 @@ syserr_eqq(VALUE self, VALUE exc) https://github.com/ruby/ruby/blob/trunk/error.c#L2599 * * fatal */ +static VALUE +exception_alloc(VALUE klass) +{ + return rb_class_allocate_instance(klass); +} + +static VALUE +exception_dumper(VALUE exc) +{ + // TODO: Currently, the instance variables "bt" and "bt_locations" + // refers to the same object (Array of String). But "bt_locations" + // should have an Array of Thread::Backtrace::Locations. + + return exc; +} + +static int +ivar_copy_i(st_data_t key, st_data_t val, st_data_t exc) +{ + rb_ivar_set((VALUE) exc, (ID) key, (VALUE) val); + return ST_CONTINUE; +} + +static VALUE +exception_loader(VALUE exc, VALUE obj) +{ + // The loader function of rb_marshal_define_compat seems to be called for two events: + // one is for fixup (r_fixup_compat), the other is for TYPE_USERDEF. + // In the former case, the first argument is an instance of Exception (because + // we pass rb_eException to rb_marshal_define_compat). In the latter case, the first + // argument is a class object (see TYPE_USERDEF case in r_object0). + // We want to copy all instance variables (but "bt_locations) from obj to exc. + // But we do not want to do so in the second case, so the following branch is for that. + if (RB_TYPE_P(exc, T_CLASS)) return obj; // maybe called from Marshal's TYPE_USERDEF + + rb_ivar_foreach(obj, ivar_copy_i, exc); + + if (rb_ivar_get(exc, id_bt) == rb_ivar_get(exc, id_bt_locations)) { + rb_ivar_set(exc, id_bt_locations, Qnil); + } + + return exc; +} + void Init_Exception(void) { rb_eException = rb_define_class("Exception", rb_cObject); + rb_define_alloc_func(rb_eException, exception_alloc); + rb_marshal_define_compat(rb_eException, rb_eException, exception_dumper, exception_loader); rb_define_singleton_method(rb_eException, "exception", rb_class_new_instance, -1); rb_define_singleton_method(rb_eException, "to_tty?", exc_s_to_tty_p, 0); rb_define_method(rb_eException, "exception", exc_exception, -1); diff --git a/test/ruby/test_marshal.rb b/test/ruby/test_marshal.rb index 1d096ad..fd62ea7 100644 --- a/test/ruby/test_marshal.rb +++ b/test/ruby/test_marshal.rb @@ -780,4 +780,35 @@ class TestMarshal < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_marshal.rb#L780 hash = Marshal.load(Marshal.dump(flagged_hash)) assert_equal(42, ruby2_keywords_test(*[hash])) end + + def exception_test + raise + end + + def test_marshal_exception + begin + exception_test + rescue => e + e2 = Marshal.load(Marshal.dump(e)) + assert_equal(e.message, e2.message) + assert_equal(e.backtrace, e2.backtrace) + assert_nil(e2.backtrace_locations) # temporal + end + end + + def nameerror_test + unknown_method + end + + def test_marshal_nameerror + begin + nameerror_test + rescue NameError => e + e2 = Marshal.load(Marshal.dump(e)) + assert_equal(e.message, e2.message) + assert_equal(e.name, e2.name) + assert_equal(e.backtrace, e2.backtrace) + assert_nil(e2.backtrace_locations) # temporal + end + end end -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/