From b7c554fbab854d0ae9b1fbbd8b676e55ffd98639 Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Thu, 4 May 2017 17:51:20 +0300 Subject: [PATCH] filters/thread_filter: don't serialise IO objects Fixes #204 (Airbrake gem segfaults on Circle CI) I stumbled upon an edge case while running the thread filter against a Rails app. The app would stuck because `io_obj.to_json` would never return. The `io_obj` was open. Closed IO objects raise IOError, which we already handle. To make sure we avoid any kind of segfaults, we ignore `:__recursive_key__`, which holds data for PrettyPrint. --- CHANGELOG.md | 3 ++ lib/airbrake-ruby/filters/thread_filter.rb | 9 ++++-- spec/filters/thread_filter_spec.rb | 32 ++++++++++++++++++++-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e91b30af..43cbff3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Airbrake Ruby Changelog ### master +* Fixed segfault on Ruby 2.1 while using the thread filter + ([#206](https://github.com/airbrake/airbrake-ruby/pull/206)) + ### [v2.2.0][v2.2.0] (May 1, 2017) * Make `notify/notify_sync` accept a block, which yields an `Airbrake::Notice` diff --git a/lib/airbrake-ruby/filters/thread_filter.rb b/lib/airbrake-ruby/filters/thread_filter.rb index 5712ca47..e6066b13 100644 --- a/lib/airbrake-ruby/filters/thread_filter.rb +++ b/lib/airbrake-ruby/filters/thread_filter.rb @@ -38,12 +38,17 @@ def call(notice) def thread_variables(th) th.thread_variables.map.with_object({}) do |var, h| - h[var] = th.thread_variable_get(var).inspect + next if (value = th.thread_variable_get(var)).is_a?(IO) + h[var] = value end end def fiber_variables(th) - th.keys.map.with_object({}) { |key, h| h[key] = th[key].inspect } + th.keys.map.with_object({}) do |key, h| + next if key == :__recursive_key__ + next if (value = th[key]).is_a?(IO) + h[key] = value + end end def add_thread_info(th, thread_info) diff --git a/spec/filters/thread_filter_spec.rb b/spec/filters/thread_filter_spec.rb index e2f02ea1..42955af7 100644 --- a/spec/filters/thread_filter_spec.rb +++ b/spec/filters/thread_filter_spec.rb @@ -11,7 +11,7 @@ subject.call(notice) th.thread_variable_set(:bingo, nil) - expect(notice[:params][:thread][:thread_variables][:bingo]).to eq(':bango') + expect(notice[:params][:thread][:thread_variables][:bingo]).to eq(:bango) end it "appends fiber variables" do @@ -19,7 +19,7 @@ subject.call(notice) th[:bingo] = nil - expect(notice[:params][:thread][:fiber_variables][:bingo]).to eq(':bango') + expect(notice[:params][:thread][:fiber_variables][:bingo]).to eq(:bango) end it "appends name", skip: !Thread.current.respond_to?(:name) do @@ -49,4 +49,32 @@ subject.call(notice) expect(notice[:params][:thread][:safe_level]).to eq(0) end + + context "when an IO-like object is stored" do + let(:io_obj) do + Class.new(IO) do + def initialize; end + end.new + end + + before do + expect(io_obj).to be_is_a(IO) + end + + it "doesn't append the IO object to thread variables" do + th.thread_variable_set(:io, io_obj) + subject.call(notice) + th.thread_variable_set(:io, nil) + + expect(notice[:params][:thread][:thread_variables][:io]).to be_nil + end + + it "doesn't append the IO object to thread variables" do + th[:io] = io_obj + subject.call(notice) + th[:io] = nil + + expect(notice[:params][:thread][:fiber_variables][:io]).to be_nil + end + end end