summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNARUSE, Yui <naruse@airemix.jp>2023-01-25 10:23:38 +0900
committerNARUSE, Yui <naruse@airemix.jp>2023-01-25 10:23:38 +0900
commit0090cb82b0bf477c29a659e34cf4427a3b1ceb27 (patch)
tree910825b3fd9d3a7f12acb2fe8633fcc135db1559
parenta20061cb1aa34b73bdbdfa7cba6cfc575a05ca38 (diff)
downloadruby-0090cb82b0bf477c29a659e34cf4427a3b1ceb27.tar.gz
merge revision(s) df6b72b8ff7af16a56fa48f3b4abb1d8850f4d1c: [Backport #19348]
Avoid checking interrupt when loading iseq MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The interrupt check will unintentionally release the VM lock when loading an iseq. And this will cause issues with the `debug` gem's [`ObjectSpace.each_iseq` method](https://github.com/ruby/debug/blob/0fcfc28acae33ec1c08068fb7c33703cfa681fa7/ext/debug/iseq_collector.c#L61-L67), which wraps iseqs with a wrapper and exposes their internal states when they're actually not ready to be used. And when that happens, errors like this would occur and kill the `debug` gem's thread: ``` DEBUGGER: ReaderThreadError: uninitialized InstructionSequence ┃ DEBUGGER: Disconnected. ┃ ["/opt/rubies/ruby-3.2.0/lib/ruby/gems/3.2.0/gems/debug-1.7.1/lib/debug/breakpoint.rb:247:in `absolute_path'", ┃ "/opt/rubies/ruby-3.2.0/lib/ruby/gems/3.2.0/gems/debug-1.7.1/lib/debug/breakpoint.rb:247:in `block in iterate_iseq'", ┃ "/opt/rubies/ruby-3.2.0/lib/ruby/gems/3.2.0/gems/debug-1.7.1/lib/debug/breakpoint.rb:246:in `each_iseq'", ... ``` A way to reproduce the issue is to satisfy these conditions at the same time: 1. `debug` gem calling `ObjectSpace.each_iseq` (e.g. [activating a `LineBreakpoint`](https://github.com/ruby/debug/blob/0fcfc28acae33ec1c08068fb7c33703cfa681fa7/lib/debug/breakpoint.rb#L246)). 2. A large amount of iseq being loaded from another thread (possibly through the `bootsnap` gem). 3. 1 and 2 iterating through the same iseq(s) at the same time. Because this issue requires external dependencies and a rather complicated timing setup to reproduce, I wasn't able to write a test case for it. But here's some pseudo code to help reproduce it: ```rb require "debug/session" Thread.new do 100.times do ObjectSpace.each_iseq do |iseq| iseq.absolute_path end end end sleep 0.1 load_a_bunch_of_iseq possibly_through_bootsnap ``` [Bug #19348] Co-authored-by: Peter Zhu <peter@peterzhu.ca> --- compile.c | 2 +- vm_core.h | 1 + vm_insnhelper.c | 11 +++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-)
-rw-r--r--compile.c2
-rw-r--r--version.h2
-rw-r--r--vm_core.h1
-rw-r--r--vm_insnhelper.c11
4 files changed, 14 insertions, 2 deletions
diff --git a/compile.c b/compile.c
index f252339ee5..c20ef48c43 100644
--- a/compile.c
+++ b/compile.c
@@ -12311,7 +12311,7 @@ ibf_load_iseq_each(struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t offset)
verify_call_cache(iseq);
RB_GC_GUARD(dummy_frame);
- rb_vm_pop_frame(ec);
+ rb_vm_pop_frame_no_int(ec);
}
struct ibf_dump_iseq_list_arg
diff --git a/version.h b/version.h
index e5f8bcfa96..74eb7be93a 100644
--- a/version.h
+++ b/version.h
@@ -11,7 +11,7 @@
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
#define RUBY_VERSION_TEENY 0
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
-#define RUBY_PATCHLEVEL 18
+#define RUBY_PATCHLEVEL 19
#include "ruby/version.h"
#include "ruby/internal/abi.h"
diff --git a/vm_core.h b/vm_core.h
index 4f6e07d818..d86fdbaecd 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -1747,6 +1747,7 @@ const VALUE *rb_binding_add_dynavars(VALUE bindval, rb_binding_t *bind, int dync
void rb_vm_inc_const_missing_count(void);
VALUE rb_vm_call_kw(rb_execution_context_t *ec, VALUE recv, VALUE id, int argc,
const VALUE *argv, const rb_callable_method_entry_t *me, int kw_splat);
+void rb_vm_pop_frame_no_int(rb_execution_context_t *ec);
MJIT_STATIC void rb_vm_pop_frame(rb_execution_context_t *ec);
void rb_thread_start_timer_thread(void);
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index f9b91a0102..8b1542bd1a 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -405,6 +405,17 @@ vm_push_frame(rb_execution_context_t *ec,
vm_push_frame_debug_counter_inc(ec, cfp, type);
}
+void
+rb_vm_pop_frame_no_int(rb_execution_context_t *ec)
+{
+ rb_control_frame_t *cfp = ec->cfp;
+
+ if (VM_CHECK_MODE >= 4) rb_gc_verify_internal_consistency();
+ if (VMDEBUG == 2) SDR();
+
+ ec->cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp);
+}
+
/* return TRUE if the frame is finished */
static inline int
vm_pop_frame(rb_execution_context_t *ec, rb_control_frame_t *cfp, const VALUE *ep)