Raise a TypeError for Thread#thread_variable{?,_get} for non-symbol

Previously, a TypeError was not raised if there were no thread
variables, because the conversion to symbol was done after that
check.  Convert to symbol before checking for whether thread
variables are set to make the behavior consistent.

Fixes [Bug #20606]

spec/ruby/core/thread/thread_variable_get_spec.rb

@@ -41,13 +41,20 @@ describe "Thread#thread_variable_get" do
     @t.thread_variable_get(:a).should be_nil
   end
 
-  it "does not raise a TypeError if the key is neither Symbol nor String, nor responds to #to_str" do
+  it "raises a TypeError if the key is neither Symbol nor String when thread variables are already set" do
-    @t.thread_variable_get(123).should be_nil
+    @t.thread_variable_set(:a, 49)
+    -> { @t.thread_variable_get(123) }.should raise_error(TypeError, "123 is not a symbol")
   end
 
-  it "does not try to convert the key with #to_sym" do
+  ruby_version_is '3.4' do
-    key = mock('key')
+    it "raises a TypeError if the key is neither Symbol nor String when no thread variables are set" do
-    key.should_not_receive(:to_sym)
+      -> { @t.thread_variable_get(123) }.should raise_error(TypeError, "123 is not a symbol")
-    @t.thread_variable_get(key).should be_nil
+    end
+
+    it "raises a TypeError if the key is neither Symbol nor String without calling #to_sym" do
+      key = mock('key')
+      key.should_not_receive(:to_sym)
+      -> { @t.thread_variable_get(key) }.should raise_error(TypeError, "#{key.inspect} is not a symbol")
+    end
   end
 end

spec/ruby/core/thread/thread_variable_spec.rb

@@ -41,13 +41,20 @@ describe "Thread#thread_variable?" do
     @t.thread_variable?(:a).should be_false
   end
 
-  it "does not raise a TypeError if the key is neither Symbol nor String, nor responds to #to_str" do
+  it "raises a TypeError if the key is neither Symbol nor String when thread variables are already set" do
-    @t.thread_variable?(123).should be_false
+    @t.thread_variable_set(:a, 49)
+    -> { @t.thread_variable?(123) }.should raise_error(TypeError, "123 is not a symbol")
   end
 
-  it "does not try to convert the key with #to_sym" do
+  ruby_version_is '3.4' do
-    key = mock('key')
+    it "raises a TypeError if the key is neither Symbol nor String when no thread variables are set" do
-    key.should_not_receive(:to_sym)
+      -> { @t.thread_variable?(123) }.should raise_error(TypeError, "123 is not a symbol")
-    @t.thread_variable?(key).should be_false
+    end
+
+    it "raises a TypeError if the key is neither Symbol nor String without calling #to_sym" do
+      key = mock('key')
+      key.should_not_receive(:to_sym)
+      -> { @t.thread_variable?(key) }.should raise_error(TypeError, "#{key.inspect} is not a symbol")
+    end
   end
 end

thread.c

@@ -3756,12 +3756,13 @@ static VALUE
 rb_thread_variable_get(VALUE thread, VALUE key)
 {
     VALUE locals;
+    VALUE symbol = rb_to_symbol(key);
 
     if (LIKELY(!THREAD_LOCAL_STORAGE_INITIALISED_P(thread))) {
         return Qnil;
     }
     locals = rb_thread_local_storage(thread);
-    return rb_hash_aref(locals, rb_to_symbol(key));
+    return rb_hash_aref(locals, symbol);
 }
 
 /*
@@ -3912,13 +3913,14 @@ static VALUE
 rb_thread_variable_p(VALUE thread, VALUE key)
 {
     VALUE locals;
+    VALUE symbol = rb_to_symbol(key);
 
     if (LIKELY(!THREAD_LOCAL_STORAGE_INITIALISED_P(thread))) {
         return Qfalse;
     }
     locals = rb_thread_local_storage(thread);
 
-    return RBOOL(rb_hash_lookup(locals, rb_to_symbol(key)) != Qnil);
+    return RBOOL(rb_hash_lookup(locals, symbol) != Qnil);
 }
 
 /*