[GR-19220] Add timeout to Queue#pop

PullRequest: truffleruby/3819

Author: andrykonchin

CHANGELOG.md

@@ -21,6 +21,7 @@ Compatibility:
 * Add `String#bytesplice` (#3039, @itarato).
 * Add `String#byteindex` and `String#byterindex` (#3039, @itarato).
 * Add implementations of `rb_proc_call_with_block`, `rb_proc_call_kw`, `rb_proc_call_with_block_kw` and `rb_funcall_with_block_kw` (#3068, @andrykonchin).
+* Add optional `timeout` argument to `Thread::Queue#pop` (#3039, @itarato).
 
 Performance:
 

lib/truffle/thread.rb

@@ -29,4 +29,15 @@ class ThreadError < StandardError
 class Thread
   Queue = ::Queue
   SizedQueue = ::SizedQueue
+
+  class Queue
+    def pop(non_block = false, timeout: nil)
+      Primitive.queue_pop(
+        self,
+        Primitive.as_boolean(non_block),
+        Truffle::QueueOperations.validate_and_prepare_timeout(non_block, timeout))
+    end
+    alias_method :shift, :pop
+    alias_method :deq, :pop
+  end
 end

lib/truffle/timeout.rb

@@ -83,15 +83,15 @@ def abort
     end
   end
 
-  @chan = Truffle::Channel.new
+  @chan = Queue.new
 
   def self.watch_channel
     reqs = []
 
     loop do
       begin
         while reqs.empty?
-          req = @chan.receive
+          req = @chan.pop
           reqs << req if req
         end
 
@@ -101,7 +101,7 @@ def self.watch_channel
         if min.left > 0
           before = Time.now
 
-          new_req = @chan.receive_timeout(min.left)
+          new_req = @chan.pop(timeout: min.left)
 
           slept_for = Time.now - before
         else

spec/ruby/core/queue/deq_spec.rb

@@ -1,6 +1,13 @@
 require_relative '../../spec_helper'
 require_relative '../../shared/queue/deque'
+require_relative '../../shared/types/rb_num2dbl_fails'
 
 describe "Queue#deq" do
   it_behaves_like :queue_deq, :deq, -> { Queue.new }
 end
+
+describe "Queue operations with timeout" do
+  ruby_version_is "3.2" do
+    it_behaves_like :rb_num2dbl_fails, nil, -> v { q = Queue.new; q.push(1); q.deq(timeout: v) }
+  end
+end

spec/ruby/core/queue/pop_spec.rb

@@ -1,6 +1,13 @@
 require_relative '../../spec_helper'
 require_relative '../../shared/queue/deque'
+require_relative '../../shared/types/rb_num2dbl_fails'
 
 describe "Queue#pop" do
   it_behaves_like :queue_deq, :pop, -> { Queue.new }
 end
+
+describe "Queue operations with timeout" do
+  ruby_version_is "3.2" do
+    it_behaves_like :rb_num2dbl_fails, nil, -> v { q = Queue.new; q.push(1); q.pop(timeout: v) }
+  end
+end

spec/ruby/core/queue/shift_spec.rb

@@ -1,6 +1,13 @@
 require_relative '../../spec_helper'
 require_relative '../../shared/queue/deque'
+require_relative '../../shared/types/rb_num2dbl_fails'
 
 describe "Queue#shift" do
   it_behaves_like :queue_deq, :shift, -> { Queue.new }
 end
+
+describe "Queue operations with timeout" do
+  ruby_version_is "3.2" do
+    it_behaves_like :rb_num2dbl_fails, nil, -> v { q = Queue.new; q.push(1); q.shift(timeout: v) }
+  end
+end

spec/ruby/shared/queue/deque.rb

@@ -37,6 +37,15 @@
     q.send(@method).should == 1
   end
 
+  it "converts false-ish for non_blocking to boolean" do
+    q = @object.call
+    q << 1
+    q << 2
+
+    q.send(@method, false).should == 1
+    q.send(@method, nil).should == 2
+  end
+
   it "returns nil for a closed empty queue" do
     q = @object.call
     q.close
@@ -143,5 +152,13 @@
       q.close
       -> { q.send(@method, true) }.should raise_error(ThreadError)
     end
+
+    it "converts true-ish non_blocking argument to true" do
+      q = @object.call
+
+      -> { q.send(@method, true) }.should raise_error(ThreadError)
+      -> { q.send(@method, 1) }.should raise_error(ThreadError)
+      -> { q.send(@method, "") }.should raise_error(ThreadError)
+    end
   end
 end

spec/ruby/shared/types/rb_num2dbl_fails.rb

@@ -0,0 +1,17 @@
+#
+# Shared tests for rb_num2dbl related conversion failures.
+#
+# Usage example:
+#   it_behaves_like :rb_num2dbl_fails, nil, -> v { o = A.new; o.foo(v) }
+#
+
+describe :rb_num2dbl_fails, shared: true do
+  it "fails if string is provided" do
+    -> { @object.call("123") }.should raise_error(TypeError, "no implicit conversion to float from string")
+  end
+
+  it "fails if boolean is provided" do
+    -> { @object.call(true) }.should raise_error(TypeError, "no implicit conversion to float from true")
+    -> { @object.call(false) }.should raise_error(TypeError, "no implicit conversion to float from false")
+  end
+end

spec/truffle/objspace/define_finalizer_spec.rb

@@ -27,17 +27,17 @@
   #   The assumption is that this works as expected but is just hard to test.
 
   it "will call the finalizer" do
-    channel = Truffle::Channel.new
+    queue = Queue.new
     finalizer = proc {
-      channel.send :finalized
+      queue << :finalized
     }
     Object.new.tap do |object|
       ObjectSpace.define_finalizer object, finalizer
       ObjectSpace.reachable_objects_from(object).should include(finalizer)
     end
     Primitive.gc_force
     Truffle::Debug.drain_finalization_queue   # Not needed for correctness
-    channel.receive_timeout(TIME_TOLERANCE).should == :finalized
+    queue.pop(timeout: TIME_TOLERANCE).should == :finalized
   end
 
 end

spec/truffle/objspace/undefine_finalizer_spec.rb

@@ -17,10 +17,10 @@
   # See comment in define_finalizer_spec.rb
 
   it "successfully unregisters a finalizer" do
-    channel = Truffle::Channel.new
+    queue = Queue.new
     Object.new.tap do |object|
       finalizer = proc {
-        channel.send :finalized
+        queue << :finalized
       }
       ObjectSpace.define_finalizer object, finalizer
       ObjectSpace.reachable_objects_from(object).should include(finalizer)
@@ -29,7 +29,7 @@
     end
     Primitive.gc_force
     Truffle::Debug.drain_finalization_queue               # Not needed for correctness
-    channel.try_receive.should be_nil
+    queue.should.empty?
   end
 
 end