Evaluate compiled blocks in an isolated Binding

Re-using the Binding of the XPath::Compiler#compile method would lead to
race conditions, and possibly a memory leak due to the Binding sticking
around for compiled Proc's lifetime.

By using a dedicated class (and its corresponding Binding) we can work
around this. Access to this class is not synchronized as compiled Procs
don't mutate their enclosing environment.

The race condition can be demonstrated using code such as the
following:

    xml = <<-EOF
    <people>
      <person>
        <name>Alice</name>
      </person>

      <person>
        <name>Bob</name>
      </person>

      <person>
        <name>Eve</name>
      </person>
    </people>
    EOF

    4.times.map do
      Thread.new do
        10_000.times do
          document = Oga.parse_xml(xml)

          document.at_xpath('people/person/name').text
        end
      end
    end.each(&:join)

Running this code would result in NoMethodErrors due to "at_xpath"
returning a NilClass opposed to an Oga::XML::Element.

lib/oga.rb

@@ -55,6 +55,7 @@ require 'oga/ruby/generator'
 
 require 'oga/xpath/lexer'
 require 'oga/xpath/parser'
+require 'oga/xpath/context'
 require 'oga/xpath/compiler'
 require 'oga/xpath/conversion'
 

lib/oga/xpath/compiler.rb

@@ -12,6 +12,11 @@ module Oga
       # @return [Oga::LRU]
       CACHE = LRU.new
 
+      # Context for compiled Procs. As compiled Procs do not mutate the
+      # enclosing environment we can just re-use the same instance without
+      # synchronization.
+      CONTEXT = Context.new
+
       # Wildcard for node names/namespace prefixes.
       STAR = '*'
 
@@ -86,7 +91,7 @@ module Oga
         generator = Ruby::Generator.new
         source    = generator.process(proc_ast)
 
-        eval(source)
+        CONTEXT.evaluate(source)
       ensure
         reset
       end

lib/oga/xpath/context.rb

@@ -0,0 +1,17 @@
+module Oga
+  module XPath
+    # Class used as the context for compiled XPath Procs.
+    #
+    # The binding of this class is used for the binding of Procs compiled by
+    # {Compiler}. Not using a specific binding would result in the procs
+    # using the binding of {Compiler#compile}, which could lead to race
+    # conditions.
+    class Context
+      # @param [String] string
+      # @return [Proc]
+      def evaluate(string)
+        binding.eval(string)
+      end
+    end # Context
+  end # XPath
+end # Oga

spec/oga/xpath/compiler_spec.rb

@@ -49,5 +49,27 @@ describe Oga::XPath::Compiler do
         block.call(doc).should be_an_instance_of(Oga::XML::NodeSet)
       end
     end
+
+    # This test relies on Binding#receiver (Binding#self on Rubinius), which
+    # sadly isn't available on all tested Ruby versions.
+    #
+    # Procs should have their own Binding as otherwise concurrent usage could
+    # lead to race conditions due to variable scopes and such being re-used.
+    [:receiver, :self].each do |binding_method|
+      if Binding.method_defined?(binding_method)
+        describe 'the returned Proc' do
+          it 'uses an isolated Binding' do
+            ast   = parse_xpath('foo')
+            block = @compiler.compile(ast)
+
+            binding_context = block.binding.send(binding_method)
+
+            binding_context.should be_an_instance_of(Oga::XPath::Context)
+          end
+        end
+
+        break
+      end
+    end
   end
 end

spec/oga/xpath/context_spec.rb

@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Oga::XPath::Context do
+  describe '#evaluate' do
+    it 'returns the result of eval()' do
+      context = described_class.new
+
+      context.evaluate('10').should == 10
+    end
+
+    describe 'assigning a variable in a lambda' do
+      it 'assigns the variable locally to the lambda' do
+        context = described_class.new
+
+        block = context.evaluate('lambda { number = 10 }')
+
+        block.call
+
+        expect { context.evaluate('number') }.to raise_error(NameError)
+      end
+    end
+  end
+end