-
Notifications
You must be signed in to change notification settings - Fork 486
Fix segfault when Ruby coverage is enabled with Rails 8.1 ERB templates #2541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
v4.2.0 introduced a fix for incorrect line numbers in stack traces on Rails 8.1 by using negative lineno values (-1) in class_eval. However, negative line numbers cause segmentation faults when Ruby's Coverage module is enabled (e.g., when using SimpleCov in CI). This is a known Ruby bug: https://bugs.ruby-lang.org/issues/19363 The fix detects whether coverage is running via Coverage.running? and adapts the lineno value accordingly: - Coverage OFF: Use -1 (correct line numbers in stack traces) - Coverage ON: Use 1 (avoids segfault, line numbers off by ~2) Note: lineno=0 was also tested but causes the same segfault, so 1 is the minimum safe value when coverage is enabled. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Question: Is there a cleaner solution?While this fix works, I wanted to raise a question about the underlying approach. The
Alternative approaches worth considering:
I'm not deeply familiar with the history here, so I wanted to ask: is the current approach (adjusting lineno) the right long-term solution, or is there a cleaner way to handle Rails 8.1 compatibility that doesn't require these workarounds? |
joelhawksley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a crack at fixing this issue! I added a couple of comments, but one key thing is missing: a test to reproduce the issue to prevent regression in the future. Please add a failing test that this PR fixes ❤️
…ineno - Remove coverage_running? check from Inline class (inline templates start at line 2+, so subtracting 1 won't result in negative line numbers) - For File templates, strip the first line of compiled source when coverage is running AND annotations are enabled, instead of using lineno=1 - Add regression tests for coverage segfault fix Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
e3511e0 to
301e650
Compare
joelhawksley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, mind looking at the CI errors?
Summary
Fixes segmentation fault when running tests with code coverage enabled (e.g., SimpleCov) on Rails 8.1+ with Ruby 3.4+.
Problem
v4.2.0 introduced negative
linenovalues (-1) inclass_evalto fix incorrect line numbers in stack traces for Rails 8.1 ERB templates. However, this causes a segmentation fault when Ruby'sCoveragemodule is enabled.Root Cause
Rails 8.1 change: Rails added annotation comments to compiled ERB output (rails/rails#53731), which added an extra line and caused stack trace line numbers to be off by 1.
v4.2.0 fix: Used
lineno = -1inclass_evalto compensate for the extra line.Ruby bug: Negative line numbers in
eval/class_evalcause segmentation faults when Ruby's Coverage module is running (bugs.ruby-lang.org/issues/19363). This bug exists in Ruby 3.4 and has been known since 2022.Result: Rails 8.1 + Ruby 3.4 + view_component 4.2.0 + SimpleCov = crash.
Solution
Detect whether coverage is running via
Coverage.running?and adapt:Why not 0? Testing showed
lineno=0also causes the segfault.1is the minimum safe value.Trade-off
This is the best possible fix given the Ruby bug. Most users (development, production) get correct line numbers. Only CI/coverage users see slightly off line numbers—but their tests actually run instead of crashing.
Fixes #2540