Properly namespace thread-local session instances

Previously I was storing the session wrapped by a
ThreadLocalSessionProxy in a static thread-local session variable,
:sunspot_session. This of course meant that two ThreadLocalSessionProxy
instances would share the same underlying Session instance in a given
thread, which is not the expected or desired behavior (e.g., when two
TLSPs are encapsulated by a MasterSlaveSessionProxy).

Now the underlying session is stored in a thread-local variable which
includes the proxy's object_id, thus scoping each session to its owning
proxy. A finalizer is used to ensure that the session instances don't
remain in the thread-local namespace after the proxy instance is garbage
collected (a concern if threads are long-lived but proxies are
short-lived; granted an unlikely use case in reality, but due diligence
never hurts).

[sunspot#103 state:resolved]

sunspot/lib/sunspot/session_proxy/thread_local_session_proxy.rb

@@ -1,3 +1,4 @@
+require 'monitor'
 require File.join(File.dirname(__FILE__), 'abstract_session_proxy')
 
 module Sunspot
@@ -8,8 +9,13 @@ module SessionProxy
     # proxy.
     #
     class ThreadLocalSessionProxy < AbstractSessionProxy
+      FINALIZER = Proc.new do |object_id|
+        Thread.current[:"sunspot_session_#{object_id}"] = nil
+      end
+
       # The configuration with which the thread-local sessions are initialized.
       attr_reader :config
+      @@next_id = 0
 
       delegate :batch, :commit, :commit_if_delete_dirty, :commit_if_dirty, :delete_dirty?, :dirty?, :index, :index!, :new_search, :remove, :remove!, :remove_all, :remove_all!, :remove_by_id, :remove_by_id!, :search, :more_like_this, :new_more_like_this, :to => :session
 
@@ -20,10 +26,11 @@ class ThreadLocalSessionProxy < AbstractSessionProxy
       #
       def initialize(config = Sunspot::Configuration.new)
         @config = config
+        ObjectSpace.define_finalizer(self, FINALIZER)
       end
 
       def session #:nodoc:
-        Thread.current[:sunspot_session] ||= Session.new(config)
+        Thread.current[:"sunspot_session_#{object_id}"] ||= Session.new(config)
       end
     end
   end

sunspot/spec/api/session_proxy/thread_local_session_proxy_spec.rb

@@ -1,4 +1,5 @@
 require File.join(File.dirname(__FILE__), 'spec_helper')
+require 'weakref'
 
 describe Sunspot::SessionProxy::ThreadLocalSessionProxy do
   before :each do
@@ -19,6 +20,22 @@
     session1.should_not eql(session2)
   end
 
+  it 'should not have the same session for the same thread in different proxy instances' do
+    proxy2 = Sunspot::SessionProxy::ThreadLocalSessionProxy.new(@config)
+    @proxy.session.should_not eql(proxy2.session)
+  end
+
+  it 'should garbage collect session instance when proxy dereferenced' do
+    ref = WeakRef.new(@proxy.session)
+    @proxy = nil
+    GC.start
+    # need to do this a second time since the reference to the session is
+    # destroyed in the finalizer during the first GC run, and thus isn't picked
+    # up by that run.
+    GC.start 
+    lambda { ref.inspect }.should raise_error(WeakRef::RefError)
+  end
+
   (Sunspot::Session.public_instance_methods(false) - ['config', :config]).each do |method|
     it "should delegate #{method.inspect} to its session" do
       args = Array.new(Sunspot::Session.instance_method(method).arity.abs) do