rspec / rspec-support

Common code needed by the other RSpec gems. Not intended for direct use.
https://rspec.info
MIT License
96 stars 104 forks source link

Mocking `Thread.current.thread_variable_get` causes stack overflow. #605

Open manueljacob opened 1 month ago

manueljacob commented 1 month ago

Mocking Thread.current.thread_variable_get used to work with rspec-support <= 3.12.0. With rspec-support >= 3.12.1, it causes a stack overflow.

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.12.0"
  ##gem "rspec-support", "3.12.0" # works
  gem "rspec-support", "3.12.1" # raises SystemStackError
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

RSpec.describe do
  it do
    expect(Thread.current).to receive(:thread_variable_get).with(:test).once.and_return(true)
    expect(Thread.current.thread_variable_get(:test)).to eq true
  end
end

Your environment

Analysis

There is infinite recursion between the following steps:

  1. Internal RSpec code calls RSpec::Support.thread_local_data.
  2. RSpec::Support.thread_local_data calls Thread.current.thread_variable_get.
  3. Because Thread.current.thread_variable_get is mocked, RSpec code is called. See 1.

Workaround

Add allow(Thread.current).to receive(:thread_variable_get).with(:__rspec).and_call_original.

The intention of the original code was to mock only Thread.current.thread_variable_get(:test) but not Thread.current.thread_variable_get called with any other arguments. However, I could not find a way to do that.

Proposed solution

If some mocked method is called from internal RSpec code, it should do nothing but delegate to the original method.

pirj commented 1 month ago

Apparently caused by https://github.com/rspec/rspec-support/pull/581 Would you like to try a different approach? Maybe stash the original method like we do for mutexes?

manueljacob commented 1 month ago

Apparently caused by #581

Yes. I had tracked down the issue to 01eb9b2a97f47202daa4947b5dc1c7046065faa0, but I forgot to mention this in the report. Sorry for that!

Would you like to try a different approach? Maybe stash the original method like we do for mutexes?

If I understood correctly, mutexes use a copy of the stdlib code. Because Thread.current.thread_variable_get is implemented in C, I changed the code to save a reference to the original methods. See #606.