Skip to content

Conversation

@luke-gruber
Copy link
Contributor

Tempfile uses DelegateClass and Tempfile should be able to be used by different ractors.

@luke-gruber luke-gruber force-pushed the delegate_class_ractors branch from f2b2c0d to 81ea986 Compare December 15, 2025 17:08
Tempfile uses DelegateClass and Tempfile should be able to be used
by different ractors.
@luke-gru
Copy link

I would like to get this in before ruby 4 release please 🙏

@hsbt hsbt merged commit 244c2c9 into ruby:master Dec 17, 2025
13 checks passed
@nobu
Copy link
Member

nobu commented Dec 17, 2025

This causes ruby 4.0 CI to crash.

Tempfile uses DelegateClass and Tempfile should be able to be used by different ractors.
Why do you want to share Tempfile across Ractors?
It sounds a very bad idea, because accessing the same I/O in multi-threading should fall into race conditions and data breakage soon.
So IO is not ractor-shareable, and also Tempfile should not be.

I claim this should be reverted, if no other rationale. (#52)

@hsbt
Copy link
Member

hsbt commented Dec 17, 2025

@luke-gru Can you describe what is the use-case of this? I'm wondering this is for your project of something.

@luke-gruber
Copy link
Contributor Author

The use case is to be able to call Tempfile.new in a ractor without it raising an exception. This is not to share Tempfiles across ractors. Currently, the first call to Tempfile.new creates the delegating methods, and it won't raise an exception there, but if another ractor tries to call Tempfile.new, it will raise.

I will look into the assertion failure in CI.

@ko1
Copy link
Contributor

ko1 commented Dec 17, 2025

Conclusion: I think it is hard to introduce it now because of essential problem.

small repro code:

class C
  define_method :gets, &(Ractor.shareable_proc do
    File.open("/tmp/foo").gets
    # $_ is assigned
  end)
end

C.new.gets
GC.verify_internal_consistency

The problem is svar (storage of $_, $~) is attached to local frame (method frame, for example. not a block frame), but svar should not be shared between Ractors. It can be explain that new assignment is used which sharable Proc shouldn't do. It is not about Tempfile/delegate, but essential issue on sharable Proc.

Some possibilities:
(1) prohibit such svar write -> Tempfile#gets doesn't work.
(2) make a sharable svar for sharable Proc -> $_ is not shareable. MatchData can be sharable.
(3) provide per-Ractor svar -> best way, but difficult in few days. The biggest issue is how to manage lifetime of the data.
(4) make svar as not method local variable, but frame local variable (available on the current block, for example) -> introduce big incompatibility
(5) provide a way to skip svar location frame (skip Delegator.delegating_block frame for svar), but it is also no time for that.

@luke-gruber
Copy link
Contributor Author

Thank you for looking into this. That makes sense, I didn't realize this svar was stored in the environment's frame. I agree that possibility 3 sounds the best as well.

@ko1
Copy link
Contributor

ko1 commented Dec 17, 2025

For delegating_block, it is another idea to use eval.

diff --git a/lib/delegate.rb b/lib/delegate.rb
index 838de675f1..79b039af48 100644
--- a/lib/delegate.rb
+++ b/lib/delegate.rb
@@ -344,11 +344,13 @@ def __setobj__(obj)
   end
 end

-def Delegator.delegating_block(mid) # :nodoc:
-  lambda do |*args, &block|
+def Delegator.delegating_code(mid)
+  <<~RUBY
+  def #{mid}(*args, &block)
     target = self.__getobj__
-    target.__send__(mid, *args, &block)
-  end.ruby2_keywords
+    target.__send__(:'#{mid}', *args, &block)
+  end
+  RUBY
 end

 #
@@ -412,11 +414,11 @@ def __setobj__(obj)  # :nodoc:
       @delegate_dc_obj = obj
     end
     protected_instance_methods.each do |method|
-      define_method(method, Delegator.delegating_block(method))
+      eval Delegator.delegating_code(method)
       protected method
     end
     public_instance_methods.each do |method|
-      define_method(method, Delegator.delegating_block(method))
+      eval Delegator.delegating_code(method)
     end
   end
   klass.define_singleton_method :public_instance_methods do |all=true|

@luke-gruber
Copy link
Contributor Author

I have made another PR for review after this one gets reverted with the suggestions from ko1.

@eregon
Copy link
Member

eregon commented Dec 17, 2025

(3) provide per-Ractor svar -> best way, but difficult in few days. The biggest issue is how to manage lifetime of the data.

This is basically what TruffleRuby already does, the svars are never shared across Threads and not even across Fibers, they are always Fiber-local conceptually. And it can still be optimized for the common case of only a single Fiber accessing the svar during a method call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants