Commit 7a5c4cd0 authored by Stan Hu's avatar Stan Hu

Fix SystemStackError when Peek bar is active with Rugged calls

Peek attempts to serialize results with `to_json`, which calls
`ActiveSupport::JSON`. If an object is passed to `to_json` that contains
instance variables, `ActiveSupport` will attempt to recursively traverse
all variables.

The problem is that we can get into an infinite loop if the instance
references to an instance that references to something else that points
back to the same instance.

To avoid this mess, we just call `to_s` on the object. It appears only
`Gitlab::Git::Repository` and `::Repository` are the culprits here.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/65404
parent ce77d137
...@@ -29,8 +29,12 @@ module Peek ...@@ -29,8 +29,12 @@ module Peek
def format_args(args) def format_args(args)
args.map do |arg| args.map do |arg|
# Needed to avoid infinite as_json calls # ActiveSupport::JSON recursively calls as_json on all
if arg.is_a?(Gitlab::Git::Repository) # instance variables, and if that instance variable points to
# something that refers back to the same instance, we can wind
# up in an infinite loop. Currently this only seems to happen with
# Gitlab::Git::Repository and ::Repository.
if arg.instance_variables.present?
arg.to_s arg.to_s
else else
arg arg
......
...@@ -24,7 +24,7 @@ describe Peek::Views::Rugged, :request_store do ...@@ -24,7 +24,7 @@ describe Peek::Views::Rugged, :request_store do
args: [project.repository.raw, 'HEAD'], args: [project.repository.raw, 'HEAD'],
duration: 0.123) duration: 0.123)
::Gitlab::RuggedInstrumentation.add_call_details(feature: :rugged_test2, ::Gitlab::RuggedInstrumentation.add_call_details(feature: :rugged_test2,
args: [project.repository.raw, 'refs/heads/master'], args: [project.repository, 'refs/heads/master'],
duration: 0.456) duration: 0.456)
results = subject.results results = subject.results
...@@ -32,7 +32,11 @@ describe Peek::Views::Rugged, :request_store do ...@@ -32,7 +32,11 @@ describe Peek::Views::Rugged, :request_store do
expect(results[:duration]).to eq('1234.00ms') expect(results[:duration]).to eq('1234.00ms')
expect(results[:details].count).to eq(2) expect(results[:details].count).to eq(2)
expect(results[:details][0][:args]).to eq([project.repository.raw.to_s, "refs/heads/master"]) expected = [
expect(results[:details][1][:args]).to eq([project.repository.raw.to_s, "HEAD"]) [project.repository.raw.to_s, "HEAD"],
[project.repository.to_s, "refs/heads/master"]
]
expect(results[:details].map { |data| data[:args] }).to match_array(expected)
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment