Implement a thread-safe Airbrake::Context

Fixes #658 (Is Airbrake.merge_context thread safe?)

This makes sure any operations on the context hash are thread-safe. We do
synchronize calls around the code that attaches the context object to the error
report but the context object is not thread-safe itself.

CHANGELOG.md

@@ -5,6 +5,8 @@ Airbrake Ruby Changelog
 
 * Improved logging on closing notifiers
   ([#644](https://github.com/airbrake/airbrake-ruby/pull/644))
+* Improved `Airbrake.merge_context` thread-safety
+  ([#659](https://github.com/airbrake/airbrake-ruby/pull/659))
 
 ### [v5.2.0][v5.2.0] (December 4, 2020)
 

lib/airbrake-ruby.rb

@@ -58,6 +58,7 @@
 require 'airbrake-ruby/monotonic_time'
 require 'airbrake-ruby/timed_trace'
 require 'airbrake-ruby/queue'
+require 'airbrake-ruby/context'
 
 # Airbrake is a thin wrapper around instances of the notifier classes (such as
 # notice, performance & deploy notifiers). It creates a way to access them via a

lib/airbrake-ruby/context.rb

@@ -0,0 +1,51 @@
+module Airbrake
+  # Represents a thread-safe Airbrake context object, which carries arbitrary
+  # information added via {Airbrake.merge_context} calls.
+  #
+  # @example
+  #   Airbrake::Context.current.merge!(foo: 'bar')
+  #
+  # @api private
+  # @since v5.2.1
+  class Context
+    # Returns current, thread-local, context.
+    # @return [self]
+    def self.current
+      Thread.current[:airbrake_context] ||= new
+    end
+
+    def initialize
+      @mutex = Mutex.new
+      @context = {}
+    end
+
+    # Merges the given context with the current one.
+    #
+    # @param [Hash{Object=>Object}] other
+    # @return [void]
+    def merge!(other)
+      @mutex.synchronize do
+        @context.merge!(other)
+      end
+    end
+
+    # @return [Hash] duplicated Hash context
+    def to_h
+      @mutex.synchronize do
+        @context.dup
+      end
+    end
+
+    # @return [Hash] clears (resets) the current context
+    def clear
+      @mutex.synchronize do
+        @context.clear
+      end
+    end
+
+    # @return [Boolean] checks whether the context has any data
+    def empty?
+      @context.empty?
+    end
+  end
+end

lib/airbrake-ruby/filters/context_filter.rb

@@ -9,19 +9,18 @@ class ContextFilter
       # @return [Integer]
       attr_reader :weight
 
-      def initialize(context)
-        @context = context
+      def initialize
         @weight = 119
         @mutex = Mutex.new
       end
 
       # @macro call_filter
       def call(notice)
         @mutex.synchronize do
-          return if @context.empty?
+          return if Airbrake::Context.current.empty?
 
-          notice[:params][:airbrake_context] = @context.dup
-          @context.clear
+          notice[:params][:airbrake_context] = Airbrake::Context.current.to_h
+          Airbrake::Context.current.clear
         end
       end
     end

lib/airbrake-ruby/notice_notifier.rb

@@ -20,14 +20,13 @@ class NoticeNotifier
 
     def initialize
       @config = Airbrake::Config.instance
-      @context = {}
       @filter_chain = FilterChain.new
       @async_sender = AsyncSender.new(:post, self.class.name)
       @sync_sender = SyncSender.new
 
       DEFAULT_FILTERS.each { |filter| add_filter(filter.new) }
 
-      add_filter(Airbrake::Filters::ContextFilter.new(@context))
+      add_filter(Airbrake::Filters::ContextFilter.new)
       add_filter(Airbrake::Filters::ExceptionAttributesFilter.new)
     end
 
@@ -79,7 +78,7 @@ def configured?
 
     # @see Airbrake.merge_context
     def merge_context(context)
-      @context.merge!(context)
+      Airbrake::Context.current.merge!(context)
     end
 
     # @return [Boolean]

spec/context_spec.rb

@@ -0,0 +1,54 @@
+RSpec.describe Airbrake::Context do
+  subject(:context) { described_class.current }
+
+  before { described_class.current.clear }
+
+  after { described_class.current.clear }
+
+  describe "#merge!" do
+    it "merges the given context with the current one" do
+      context.merge!(apples: 'oranges')
+      expect(context.to_h).to match(apples: 'oranges')
+    end
+  end
+
+  describe "#clear" do
+    it "clears the context" do
+      context.merge!(apples: 'oranges')
+      context.clear
+      expect(context.to_h).to be_empty
+    end
+  end
+
+  describe "#to_h" do
+    it "returns a hash representation of the context" do
+      expect(context.to_h).to be_a(Hash)
+    end
+  end
+
+  describe "#empty?" do
+    context "when the context has data" do
+      it "returns true" do
+        context.merge!(apples: 'oranges')
+        expect(context).not_to be_empty
+      end
+    end
+
+    context "when the context has NO data" do
+      it "returns false" do
+        expect(context).to be_empty
+      end
+    end
+  end
+
+  context "when another thread is spawned" do
+    it "doesn't clash with other threads' contexts" do
+      described_class.current.merge!(apples: 'oranges')
+      th = Thread.new do
+        described_class.current.merge!(foos: 'bars')
+      end
+      th.join
+      expect(described_class.current.to_h).to match(apples: 'oranges')
+    end
+  end
+end

spec/filters/context_filter_spec.rb

@@ -3,21 +3,30 @@
 
   context "when the current context is empty" do
     it "doesn't merge anything with params" do
-      described_class.new({}).call(notice)
+      described_class.new.call(notice)
       expect(notice[:params]).to be_empty
     end
   end
 
   context "when the current context has some data" do
     it "merges the data with params" do
-      described_class.new(apples: 'oranges').call(notice)
+      Airbrake.merge_context(apples: 'oranges')
+      described_class.new.call(notice)
       expect(notice[:params]).to eq(airbrake_context: { apples: 'oranges' })
     end
 
-    it "clears the data from the provided context" do
+    it "clears the data from the current context" do
       context = { apples: 'oranges' }
-      described_class.new(context).call(notice)
-      expect(context).to be_empty
+      Airbrake.merge_context(context)
+      described_class.new.call(notice)
+      expect(Airbrake::Context.current).to be_empty
+    end
+
+    it "does not mutate the provided context object" do
+      context = { apples: 'oranges' }
+      Airbrake.merge_context(context)
+      described_class.new.call(notice)
+      expect(context).to match(apples: 'oranges')
     end
   end
 end