Skip to content

perf: cache backtrace line parsing and Line object creation #2905

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.

Sign up for GitHub

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

Open
HazAT wants to merge 1 commit into
base: master
Choose a base branch
from
+45 −18
Open

perf: cache backtrace line parsing and Line object creation #2905

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 45 additions & 18 deletions sentry-ruby/lib/sentry/backtrace/line.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,57 @@ class Line

attr_reader :in_app_pattern

# Cache parsed Line data (file, number, method, module_name) by unparsed line string.
# Same backtrace lines appear repeatedly (same code paths, same errors).
# Values are frozen arrays to avoid mutation.
# Limited to 2048 entries to prevent unbounded memory growth.
PARSE_CACHE_LIMIT = 2048
@parse_cache = {}

# Cache complete Line objects by (unparsed_line, in_app_pattern) to avoid
# re-creating identical Line objects across exceptions.
@line_object_cache = {}
Comment on lines +39 to +43
Copy link
Copy Markdown

@sentry sentry bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The new caching mechanisms in Backtrace::Line use non-thread-safe Hash objects, creating race conditions in multi-threaded environments during cache read, write, and clear operations.
Severity: MEDIUM

Suggested Fix

Replace the plain Hash objects used for @parse_cache and @line_object_cache with a thread-safe alternative. Since the project already depends on concurrent-ruby, using Concurrent::Map is the recommended approach, as it is already used for other caches in the codebase.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: sentry-ruby/lib/sentry/backtrace/line.rb#L38-L42

Potential issue: The new caching mechanisms (`@parse_cache`, `@line_object_cache`) in
`Backtrace::Line` use plain `Hash` objects, which are not thread-safe. In a
multi-threaded environment, concurrent read/write operations can lead to race
conditions. Specifically, the non-atomic sequence of checking the cache size, clearing
it, and then writing a new entry can result in lost entries and an inconsistent cache
state. This can degrade cache effectiveness and potentially cause errors on non-GIL Ruby
implementations like JRuby.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown

@cursor cursor bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thread-unsafe plain Hash caches risk corruption on JRuby

Medium Severity

The three new class-level caches (@parse_cache, @line_object_cache, @in_app_pattern_cache) use plain {} hashes, but they're accessed concurrently from multiple threads during exception capture. On JRuby (which the SDK explicitly supports via JAVA_INPUT_FORMAT), there is no GIL, and concurrent reads/writes to a plain Hash can corrupt its internal structure. The existing line_cache in the same file already uses Concurrent::Map for this exact reason.

Additional Locations (1)
Fix in Cursor Fix in Web


# Parses a single line of a given backtrace
# @param [String] unparsed_line The raw line from +caller+ or some backtrace
# @return [Line] The parsed backtrace line
def self.parse(unparsed_line, in_app_pattern = nil)
ruby_match = unparsed_line.match(RUBY_INPUT_FORMAT)

if ruby_match
file = ruby_match[1]
number = ruby_match[2]
module_name = ruby_match[4]
method = ruby_match[5]
if file.end_with?(CLASS_EXTENSION)
file.sub!(/\.class$/, RB_EXTENSION)
end
else
java_match = unparsed_line.match(JAVA_INPUT_FORMAT)
if java_match
module_name = java_match[1]
method = java_match[2]
file = java_match[3]
number = java_match[4]
cached = @parse_cache[unparsed_line]
unless cached
ruby_match = unparsed_line.match(RUBY_INPUT_FORMAT)

if ruby_match
file = ruby_match[1]
number = ruby_match[2]
module_name = ruby_match[4]
method = ruby_match[5]
if file.end_with?(CLASS_EXTENSION)
file.sub!(/\.class$/, RB_EXTENSION)
end
else
java_match = unparsed_line.match(JAVA_INPUT_FORMAT)
if java_match
module_name = java_match[1]
method = java_match[2]
file = java_match[3]
number = java_match[4]
end
end
cached = [file, number, method, module_name].freeze
@parse_cache.clear if @parse_cache.size >= PARSE_CACHE_LIMIT
@parse_cache[unparsed_line] = cached
end
new(file, number, method, module_name, in_app_pattern)

line = new(cached[0], cached[1], cached[2], cached[3], in_app_pattern)

# Cache the Line object — limited by parse cache limit
if @line_object_cache.size >= PARSE_CACHE_LIMIT
@line_object_cache.clear
end
pattern_cache = (@line_object_cache[object_cache_key] ||= {})
Copy link
Copy Markdown

@sentry sentry bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The variable object_cache_key is used without being defined, which will raise a NameError at runtime and crash the method.
Severity: CRITICAL

Suggested Fix

Replace the undefined variable object_cache_key on line 81 with the unparsed_line variable, which is the intended cache key.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: sentry-ruby/lib/sentry/backtrace/line.rb#L81

Potential issue: In the `Backtrace::Line.parse` method, line 81 references a local
variable `object_cache_key` that is never assigned a value. This will cause a
`NameError` every time the method is called, preventing the Sentry SDK from capturing
any exceptions. The variable appears to be a typo and was likely intended to be
`unparsed_line`, which is a parameter to the method. This crash makes the associated
caching logic non-functional.

Copy link
Copy Markdown

@cursor cursor bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undefined method object_cache_key causes runtime error

High Severity

object_cache_key on line 81 is never defined anywhere — not as a method, variable, or constant. Every call to Line.parse will raise a NoMethodError. This appears to be an incomplete refactor; the intended key was likely unparsed_line.

Fix in Cursor Fix in Web

pattern_cache[in_app_pattern] = line

line
Copy link
Copy Markdown

@cursor cursor bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line object cache is written but never read

Medium Severity

The @line_object_cache is populated on every call but never consulted before creating a new Line object on line 75. A cache lookup (and early return on hit) is missing, so every invocation still allocates a new Line, defeating the stated purpose of the object cache. The cache currently only wastes memory.

Fix in Cursor Fix in Web

end

# Creates a Line from a Thread::Backtrace::Location object
Expand Down
Loading