Avoid checking interrupt when loading iseq

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](0fcfc28aca/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`](0fcfc28aca/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>
This commit is contained in:
Stan Lo 2023-01-16 20:42:51 +00:00 committed by Peter Zhu
parent 9399352a43
commit df6b72b8ff
3 changed files with 13 additions and 1 deletions

View File

@ -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

View File

@ -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);

View File

@ -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)