Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Sep 10, 2021

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull bot added the ⤵️ pull label Sep 10, 2021
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Asked ko1 about the design.  [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
Must not be a bad idea to improve documents. [ci skip]
matzbot and others added 26 commits September 16, 2021 03:37
If $LOADED_FEATURES is changed in the just required file, also the
index table needs to be updated before loaded_features_snapshot is
reset.  If the snapshot is reset without updating the table, the
name of the added feature will not be found.
The method to return the single matched substring corresponding to
the given argument.
The method to return the length of the matched substring
corresponding to the given argument.
mutex_mark is (basically) NULL, so we don't have any references to mark.
This means we should safely be able to mark Mutex as WB_PROTECTED
without changing anything else.
Treated:

    #to_s
    #inspect
    #===
    #include?
    #cover?
    #count
This fixes issues with paths being loaded twice in certain cases
when symlinks are used.

It took me multiple attempts to get this working.  My original
attempt tried to convert paths to realpaths before adding them
to $LOADED_FEATURES.  Unfortunately, this doesn't work well
with the loaded feature index, which is based off load paths
and not realpaths. While I was able to get require working, I'm
fairly sure the loaded feature index was not being used as
expected, which would have significant performance implications.
Additionally, I was never able to get that approach working with
autoload when autoloading a non-realpath file. It also broke
some specs.

This takes a more conservative approach. Directly before loading the
file, if the file with the same realpath has been required, the
loading of the file is skipped. The realpaths are stored as
fstrings in a hidden hash.

When rebuilding the loaded feature index, the hash of realpaths
is also rebuilt.  I'm guessing this makes rebuilding process
slower, but I don think that is a hot path. In general, modifying
loaded features is only done when reloading, and that tends to be
in non-production environments.

Change test_require_with_loaded_features_pop test to use 30 threads
and 300 iterations, instead of 4 threads and 1000 iterations.
I saw only sporadic failures with 4/1000, but consistent failures
30/300 threads. These failures were due to the fact that the
concurrent deletions from $LOADED_FEATURES in other threads can
result in rb_ary_entry returning nil when rebuilding the loaded
features index.

To avoid concurrency issues when rebuilding the loaded features
index, the building of the index itself is left alone, and
afterwards, a separate loop is done on a copy of the loaded feature
snapshot in order to rebuild the realpaths hash.

Fixes [Bug #17885]
@devkadirselcuk devkadirselcuk merged commit 6fd4d71 into turkdevops:master Sep 18, 2021
pull bot pushed a commit that referenced this pull request Aug 26, 2025
If we malloc when the current Ractor is locked, we can deadlock because
GC requires VM lock and Ractor barrier. If another Ractor is waiting on
this Ractor lock, then it will deadlock because the other Ractor will
never join the barrier.

For example, this script deadlocks:

    r = Ractor.new do
      loop do
        Ractor::Port.new
      end
    end

    100000.times do |i|
      r.send(nil)
      puts i
    end

On debug builds, it fails with this assertion error:

    vm_sync.c:75: Assertion Failed: vm_lock_enter:cr->sync.locked_by != rb_ractor_self(cr)

On non-debug builds, we can see that it deadlocks in the debugger:

    Main Ractor:
    frame #3: 0x000000010021fdc4 miniruby`rb_native_mutex_lock(lock=<unavailable>) at thread_pthread.c:115:14
    frame #4: 0x0000000100193eb8 miniruby`ractor_send0 [inlined] ractor_lock(r=<unavailable>, file=<unavailable>, line=1180) at ractor.c:73:5
    frame #5: 0x0000000100193eb0 miniruby`ractor_send0 [inlined] ractor_send_basket(ec=<unavailable>, rp=0x0000000131092840, b=0x000000011c63de80, raise_on_error=true) at ractor_sync.c:1180:5
    frame #6: 0x0000000100193eac miniruby`ractor_send0(ec=<unavailable>, rp=0x0000000131092840, obj=4, move=<unavailable>, raise_on_error=true) at ractor_sync.c:1211:5

    Second Ractor:
    frame #2: 0x00000001002208d0 miniruby`rb_ractor_sched_barrier_start [inlined] rb_native_cond_wait(cond=<unavailable>, mutex=<unavailable>) at thread_pthread.c:221:13
    frame #3: 0x00000001002208cc miniruby`rb_ractor_sched_barrier_start(vm=0x000000013180d600, cr=0x0000000131093460) at thread_pthread.c:1438:13
    frame #4: 0x000000010028a328 miniruby`rb_vm_barrier at vm_sync.c:262:13 [artificial]
    frame #5: 0x00000001000dfa6c miniruby`gc_start [inlined] rb_gc_vm_barrier at gc.c:179:5
    frame #6: 0x00000001000dfa68 miniruby`gc_start [inlined] gc_enter(objspace=0x000000013180fc00, event=gc_enter_event_start, lock_lev=<unavailable>) at default.c:6636:9
    frame #7: 0x00000001000dfa48 miniruby`gc_start(objspace=0x000000013180fc00, reason=<unavailable>) at default.c:6361:5
    frame #8: 0x00000001000e3fd8 miniruby`objspace_malloc_increase_body [inlined] garbage_collect(objspace=0x000000013180fc00, reason=512) at default.c:6341:15
    frame #9: 0x00000001000e3fa4 miniruby`objspace_malloc_increase_body [inlined] garbage_collect_with_gvl(objspace=0x000000013180fc00, reason=512) at default.c:6741:16
    frame #10: 0x00000001000e3f88 miniruby`objspace_malloc_increase_body(objspace=0x000000013180fc00, mem=<unavailable>, new_size=<unavailable>, old_size=<unavailable>, type=<unavailable>) at default.c:8007:13
    frame #11: 0x00000001000e3c44 miniruby`rb_gc_impl_malloc [inlined] objspace_malloc_fixup(objspace=0x000000013180fc00, mem=0x000000011c700000, size=12582912) at default.c:8085:5
    frame #12: 0x00000001000e3c30 miniruby`rb_gc_impl_malloc(objspace_ptr=0x000000013180fc00, size=12582912) at default.c:8182:12
    frame #13: 0x00000001000d4584 miniruby`ruby_xmalloc [inlined] ruby_xmalloc_body(size=<unavailable>) at gc.c:5128:12
    frame #14: 0x00000001000d4568 miniruby`ruby_xmalloc(size=<unavailable>) at gc.c:5118:34
    frame #15: 0x00000001001eb184 miniruby`rb_st_init_existing_table_with_size(tab=0x000000011c2b4b40, type=<unavailable>, size=<unavailable>) at st.c:559:39
    frame #16: 0x00000001001ebc74 miniruby`rebuild_table_if_necessary [inlined] rb_st_init_table_with_size(type=0x00000001004f4a78, size=524287) at st.c:585:5
    frame #17: 0x00000001001ebc5c miniruby`rebuild_table_if_necessary [inlined] rebuild_table(tab=0x000000013108e2f0) at st.c:753:19
    frame #18: 0x00000001001ebbfc miniruby`rebuild_table_if_necessary(tab=0x000000013108e2f0) at st.c:1125:9
    frame #19: 0x00000001001eba08 miniruby`rb_st_insert(tab=0x000000013108e2f0, key=262144, value=4767566624) at st.c:1143:5
    frame #20: 0x0000000100194b84 miniruby`ractor_port_initialzie [inlined] ractor_add_port(r=0x0000000131093460, id=262144) at ractor_sync.c:399:9
    frame #21: 0x0000000100194b58 miniruby`ractor_port_initialzie [inlined] ractor_port_init(rpv=4750065560, r=0x0000000131093460) at ractor_sync.c:87:5
    frame #22: 0x0000000100194b34 miniruby`ractor_port_initialzie(self=4750065560) at ractor_sync.c:103:12
pull bot pushed a commit that referenced this pull request Nov 7, 2025
We were seeing errors like:

```
* thread #8, stop reason = EXC_BAD_ACCESS (code=1, address=0x803)
  * frame #0: 0x00000001001fe944 ruby`rb_st_lookup(tab=0x00000000000007fb, key=1, value=0x00000001305b7490) at st.c:1066:22
    frame #1: 0x000000010002d658 ruby`remove_class_from_subclasses [inlined] class_get_subclasses_for_ns(tbl=0x00000000000007fb, ns_id=1) at class.c:604:9
    frame #2: 0x000000010002d650 ruby`remove_class_from_subclasses(tbl=0x00000000000007fb, ns_id=1, klass=4754039232) at class.c:620:34
    frame #3: 0x000000010002c8a8 ruby`rb_class_classext_free_subclasses(ext=0x000000011b5ce1d8, klass=4754039232, replacing=<unavailable>) at class.c:700:9
    frame #4: 0x000000010002c760 ruby`rb_class_classext_free(klass=4754039232, ext=0x000000011b5ce1d8, is_prime=true) at class.c:105:5
    frame #5: 0x00000001000e770c ruby`classext_free(ext=<unavailable>, is_prime=<unavailable>, namespace=<unavailable>, arg=<unavailable>) at gc.c:1231:5 [artificial]
    frame #6: 0x000000010002d178 ruby`rb_class_classext_foreach(klass=<unavailable>, func=(ruby`classext_free at gc.c:1228), arg=0x00000001305b75c0) at class.c:518:5
    frame #7: 0x00000001000e745c ruby`rb_gc_obj_free(objspace=0x000000012500c400, obj=4754039232) at gc.c:1282:9
    frame #8: 0x00000001000e70d4 ruby`gc_sweep_plane(objspace=0x000000012500c400, heap=<unavailable>, p=4754039232, bitset=4095, ctx=0x00000001305b76e8) at default.c:3482:21
    frame #9: 0x00000001000e6e9c ruby`gc_sweep_page(objspace=0x000000012500c400, heap=0x000000012500c540, ctx=0x00000001305b76e8) at default.c:3567:13
    frame #10: 0x00000001000e51d0 ruby`gc_sweep_step(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:3848:9
    frame #11: 0x00000001000e1880 ruby`gc_continue [inlined] gc_sweep_continue(objspace=0x000000012500c400, sweep_heap=0x000000012500c540) at default.c:3931:13
    frame #12: 0x00000001000e1754 ruby`gc_continue(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:2037:9
    frame #13: 0x00000001000e10bc ruby`newobj_cache_miss [inlined] heap_prepare(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:2056:5
    frame #14: 0x00000001000e1074 ruby`newobj_cache_miss [inlined] heap_next_free_page(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:2280:9
    frame #15: 0x00000001000e106c ruby`newobj_cache_miss(objspace=0x000000012500c400, cache=0x0000600001b00300, heap_idx=2, vm_locked=false) at default.c:2387:38
    frame #16: 0x00000001000e0d28 ruby`newobj_alloc(objspace=<unavailable>, cache=<unavailable>, heap_idx=<unavailable>, vm_locked=<unavailable>) at default.c:2411:15 [artificial]
    frame #17: 0x00000001000d7214 ruby`newobj_of [inlined] rb_gc_impl_new_obj(objspace_ptr=<unavailable>, cache_ptr=<unavailable>, klass=<unavailable>, flags=<unavailable>, wb_protected=<unavailable>, alloc_size=<unavailable>) at default.c:2490:15
    frame #18: 0x00000001000d719c ruby`newobj_of(cr=<unavailable>, klass=4313971728, flags=258, wb_protected=<unavailable>, size=<unavailable>) at gc.c:995:17
    frame #19: 0x00000001000d73ec ruby`rb_wb_protected_newobj_of(ec=<unavailable>, klass=<unavailable>, flags=<unavailable>, size=<unavailable>) at gc.c:1044:12 [artificial]
    frame #20: 0x0000000100032d34 ruby`class_alloc0(type=<unavailable>, klass=4313971728, namespaceable=<unavailable>) at class.c:803:5
```
pull bot pushed a commit that referenced this pull request Nov 11, 2025
We don't decrement the super and module subclasses count for iclasses that
are having their classext replaced. This causes the reference count to be
incorrect and leak memory.

The following script demonstrates the memory leak:

    module Foo
      refine(Object) do
        define_method(:<=) {}
      end
    end

    class Bar
      include Comparable
    end

With RUBY_FREE_AT_EXIT and ASAN, we can see many memory leaks, including:

    Direct leak of 16 byte(s) in 1 object(s) allocated from:
        #0 0x599f715adca2 in calloc (miniruby+0x64ca2)
        #1 0x599f716bd779 in calloc1 gc/default/default.c:1495:12
        #2 0x599f716d1370 in rb_gc_impl_calloc gc/default/default.c:8216:5
        #3 0x599f716b8ab1 in ruby_xcalloc_body gc.c:5221:12
        #4 0x599f716b269c in ruby_xcalloc gc.c:5215:34
        #5 0x599f715eab23 in class_alloc0 class.c:790:22
        #6 0x599f715e4bec in class_alloc class.c:836:12
        #7 0x599f715e60c9 in module_new class.c:1693:17
        #8 0x599f715e60a2 in rb_module_new class.c:1701:12
        #9 0x599f715e6303 in rb_define_module class.c:1733:14
        #10 0x599f715ebc5f in Init_Comparable compar.c:315:22
        #11 0x599f716e35f5 in rb_call_inits inits.c:32:5
        #12 0x599f7169cbfd in ruby_setup eval.c:88:9
        #13 0x599f7169cdac in ruby_init eval.c:100:17
        #14 0x599f715b0fa9 in rb_main main.c:41:5
        #15 0x599f715b0f59 in main main.c:62:12
        #16 0x739b2f02a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #17 0x739b2f02a28a in __libc_start_main csu/../csu/libc-start.c:360:3
        #18 0x599f7157c424 in _start (miniruby+0x33424)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.