Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Sep 26, 2025

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

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

amaxcz and others added 13 commits September 25, 2025 16:03
Mutexes were being improperly unlocked on thread cleanup. This bug was
introduced in 050a895.

We must keep a reference from the mutex to the thread, because if the fiber
is collected before the mutex is, then we cannot unlink it from the thread in
`mutex_free`. If it's not unlinked from the the thread when it's freed, it
causes bugs in `rb_thread_unlock_all_locking_mutexes`.

We now mark the fiber when a mutex is locked, and the thread is marked
as well. However, a fiber can still be freed in the same GC cycle as the
mutex, so the reference to the thread is still needed.

The reason we need to mark the fiber is that `mutex_owned_p()` has an ABA
issue where if the fiber is collected while it's locked, a new fiber could be
allocated at the same memory address and we could get false positives.

Fixes [Bug #21342]

Co-authored-by: John Hawthorn <john@hawthorn.email>
rb_profile_frames() is used by profilers in a way such that it can run
on any instruction in the binary, and it crashed previously in the
following situation in `RUBY_DEBUG` builds:

```
* thread #1, queue = 'com.apple.main-thread', stop reason = step over
    frame #0: 0x00000001002827f0 miniruby`vm_make_env_each(ec=0x0000000101866b00, cfp=0x000000080c91bee8) at vm.c:992:74
   989              }
   990
   991              vm_make_env_each(ec, prev_cfp);
-> 992              VM_FORCE_WRITE_SPECIAL_CONST(&ep[VM_ENV_DATA_INDEX_SPECVAL], VM_GUARDED_PREV_EP(prev_cfp->ep));
   993          }
   994      }
   995      else {
(lldb) call rb_profile_frames(0, 100, $2, $3)
/Users/alan/ruby/vm_core.h:1448: Assertion Failed: VM_ENV_FLAGS:FIXNUM_P(flags)
ruby 3.5.0dev (2025-09-23T20:20:04Z master 06b7a70) +PRISM [arm64-darwin25]

-- Crash Report log information --------------------------------------------
   See Crash Report log file in one of the following locations:
     * ~/Library/Logs/DiagnosticReports
     * /Library/Logs/DiagnosticReports
   for more details.
Don't forget to include the above Crash Report log file in bug reports.

-- Control frame information -----------------------------------------------
c:0008 p:---- s:0029 e:000028 CFUNC  :lambda
/Users/alan/ruby/vm_core.h:1448: Assertion Failed: VM_ENV_FLAGS:FIXNUM_P(flags)
ruby 3.5.0dev (2025-09-23T20:20:04Z master 06b7a70) +PRISM [arm64-darwin25]

-- Crash Report log information --------------------------------------------
<snip>
```

There is a small window where the control frame is invalid and fails the
assert.

The double crash also shows that in `RUBY_DEBUG` builds, the crash reporter was
previously not resilient to corrupt frame state. In release builds, it
prints more info.

Add unchecked APIs for the crash reporter and profilers so they work
as well in `RUBY_DEBUG` builds as non-debug builds.
The old code was doing a manual HashSet/HashMap rebuild, and there isn't
a clear performance advantage over `Iterator::map`. So let's use `map`
since it looks clearer and it's easier to see that everything was indeed
updated. This also adds assertions the old code did not have by way
of as_iseq() and as_cme().
Without this, we crash during reference update.
A little follow-up on #14653

Now that we don't generate a PC-less side exit at the entry block, we
shouldn't need this guard that was added by #14643.
@pull pull bot locked and limited conversation to collaborators Sep 26, 2025
@pull pull bot added the ⤵️ pull label Sep 26, 2025
@pull pull bot merged commit cc1fd64 into turkdevops:master Sep 26, 2025
1 check failed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants