この記事は、Ruby Advent Calendar 2020 の 8 日目の記事です。
昨日は @udzura さんの mruby 本を支えた Continuous Delivery でした。

概要

Thread#raise を使う場合に ensure が実行されないケースがあります。
代表例としては Timeout.timeout があります。

Rails をはじめとするライブラリはクリーンアップ処理を ensure に頼っているケースが多いので、リクエスト中に Timeout.timeout した場合、そのプロセスは後続リクエストを正常に処理できない可能性があります。
次のリクエストを処理する前に終了させたほうがよいです。

rack middleware やライブラリ内でこれらを使用していないかには注意したほうがよいでしょう。
例えば、rack-timeout を使ってデフォルト設定のままの場合には、必ず term_on_timeout: true にしましょう。

詳細

次のコードを実行してみてください。
ensure できなかった場合にのみ無限ループから抜けます。

require 'timeout'

class Foo
  def stack
    @stack ||= []
  end

  def foo
    stack.push('hi')
  ensure
    stack.pop
  end
end

foo = nil
loop do
  foo = Foo.new
  begin
    Timeout.timeout(Float::EPSILON) do
      loop do
        foo.foo
      end
    end
  rescue Timeout::Error
    break unless foo.stack.empty?
  end
end

p foo.stack
# => ["hi"]

まさにこのような場合でも確実に ensure するために Thread.handle_interrupt があります。

require 'timeout'

class Foo
  def stack
    @stack ||= []
  end

  def foo
    Thread.handle_interrupt(Timeout::Error => :never) do
      begin
        stack.push('hi')
      ensure
        stack.pop
      end
    end
  end
end

foo = nil
loop do
  foo = Foo.new
  begin
    Timeout.timeout(Float::EPSILON) do
      loop do
        foo.foo
      end
    end
  rescue Timeout::Error
    break unless foo.stack.empty?
  end
end

p foo.stack
# ここまでこない

つまり、ensure の位置でこの Thread.handle_interrupt をアプリケーションコードとライブラリのコードすべてに書いて回る必要があります。もっと言うと、すべての行でいつでも例外が上がってくるかもしれないということなので、そのようなことが起きても内部状態が壊れないよう考慮したコーディングが必要になります。

これについて、Rails ではきりがないため考慮しないこととしています。

ところで、ActiveRecord は scope 周りで ensure を利用している部分があります。

これを利用して、Timeout 前のリクエストで投げようとしていたクエリの条件を Timeout 後のスコープに入れ込むことが可能になります。

つまり、

User.count

というコードを、

User.where(name: "foo").count

に化けさせることができます。

以下が再現させるコードの例です。
保存して $ ruby foo.rb のようにすればそのまま実行できます。

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  gem "rails", "6.0.3"
  gem "sqlite3"
  gem "rack-timeout", require: "rack/timeout/base"
end

require "rack/test"
require "active_record/railtie"
require "action_controller/railtie"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(File::NULL)
ActiveRecord::Base.logger.level = Logger::ERROR

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.string :name
  end
end

class User < ActiveRecord::Base
  scope :sleep_a_while, -> { all.tap { sleep(rand / 1000000.0) } }
end

User.create!(name: "foo")
User.create!(name: "bar")

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts << "example.org"
  config.session_store :cookie_store, key: "cookie_store_key"
  secrets.secret_key_base = "secret_key_base"

  config.logger = ActiveRecord::Base.logger
  Rails.logger  = config.logger

  config.middleware.insert_before Rack::Runtime, Rack::Timeout, service_timeout: 0.1

  config.exceptions_app = ->(*) { [500, {}, ['']] }

  routes.draw do
    get "/" => "test#index"
    get "/timeout" => "test#timeout"
  end
end

class TestController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    render plain: User.count
  end

  def timeout
    loop do
      User.where(name: "foo").sleep_a_while
    end
  end
end

require "minitest/autorun"

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_index
    get "/"

    # User.count が 2 であることを確認
    assert_equal "2", last_response.body

    until last_response.body == "1"
      get "/timeout"

      get "/"
    end

    get "/"
    # User.count の結果は常に 2 でないといけないが、1 になっているためテストは失敗する
    assert_equal "2", last_response.body
  end

  private
    def app
      Rails.application
    end
end

このように、ensure に頼って書かれているコード全般が Timeout.timeout と組み合わせることで壊れる可能性があります。

その他

8年前に ko1 さんが ensure と Timeout.timeout の組み合わせの危険性についてアドベントカレンダーで書かれていました。
http://atdot.net/~ko1/diary/edit_comment.cgi?mode=long&year=2012&month=12&day=6

また、rack-timeout ではこういった timeout のリスクについて記しています。
https://github.com/sharpstone/rack-timeout/blob/f4b14a534b37d425ec4dba10d9cacf29ba012f9f/doc/risks.md

とはいえ、rack-timeout を使うにあたってはデフォルトで term_on_timeout: true のほうが望ましいと思われるので issue を立てています。
https://github.com/sharpstone/rack-timeout/issues/169