Merge pull request oracle#436 in G/truffleruby from queue to master

* commit '5f3a55dc82d87d7535c65b70efb00444b8a974a6': (34 commits)
  Change log entry for locking fast path
  The timeout parameter in BlockingMatcher was dead code for a while
  Handle exceptions in BlockingMatcher
  Made BlockingMatcher more stable
  Fix queue closed semantics
  Missing boundaries
  Reorder
  Fix missing unlock
  Fix mutual exclusion specs
  Fix Thread::Queue specs and implement
  Fix closing semantics in queues
  Restructured closed tests
  UnsizedQueue.pop(true) never returns CLOSED
  Remove dead mutex
  Join threads created in specs
  Formatting
  Reorder code
  More specs for queues
  Spec Thread::Queue etc by just saying it’s the same class as ::Queue
  Improve some synchronisation specs
  ...

CHANGELOG.md

@@ -8,6 +8,13 @@ Bug fixes:
 
 * Added `rb_eEncodingError` for C extensions (#1437).
 * Fixed race condition when creating threads (#1445).
+# 1.0
+
+New features
+
+* `Mutex` and `ConditionVariable` have a new fast path for acquiring locks
+  that are unlocked.
+* `Queue` and `SizedQueue`, `#close` and `#closed?`, have been implemented.
 
 # 1.0 RC 9
 

lib/truffle/thread.rb

@@ -24,3 +24,8 @@ class ThreadError < StandardError
 
 # Truffle: ConditionVariable is defined in the core library.
 #          Queue and SizedQueue are defined in Java
+
+class Thread
+  Queue = ::Queue
+  SizedQueue = ::SizedQueue
+end

spec/mspec/lib/mspec/matchers/block_caller.rb

@@ -1,26 +1,24 @@
 class BlockingMatcher
   def matches?(block)
-    started = false
-    blocking = true
-
-    thread = Thread.new do
-      started = true
+    t = Thread.new do
       block.call
-
-      blocking = false
     end
 
-    Thread.pass while !started
-
-    # Wait until the Thread status is "sleep" (then it's blocking)
-    # or nil (the Thread finished execution, it did not block)
-    while status = thread.status and status != "sleep"
-      Thread.pass
+    loop do
+      case t.status
+      when "sleep"    # blocked
+        t.kill
+        t.join
+        return true
+      when false      # terminated normally, so never blocked
+        t.join
+        return false
+      when nil        # terminated exceptionally
+        t.value
+      else
+        Thread.pass
+      end
     end
-    thread.kill
-    thread.join
-
-    blocking
   end
 
   def failure_message
@@ -33,7 +31,7 @@ def negative_failure_message
 end
 
 module MSpecMatchers
-  private def block_caller(timeout = 0.1)
+  private def block_caller
     BlockingMatcher.new
   end
 end

spec/ruby/core/mutex/lock_spec.rb

@@ -11,22 +11,15 @@
     m.unlock
   end
 
-  it "waits if the lock is not available" do
+  it "blocks the caller if already locked" do
     m = Mutex.new
-
     m.lock
+    lambda { m.lock }.should block_caller
+  end
 
-    th = Thread.new do
-      m.lock
-      ScratchPad.record :after_lock
-    end
-
-    Thread.pass while th.status and th.status != "sleep"
-
-    ScratchPad.recorded.should be_nil
-    m.unlock
-    th.join
-    ScratchPad.recorded.should == :after_lock
+  it "does not block the caller if not locked" do
+    m = Mutex.new
+    lambda { m.lock }.should_not block_caller
   end
 
   # Unable to find a specific ticket but behavior change may be

spec/ruby/core/mutex/synchronize_spec.rb

@@ -24,4 +24,43 @@
     th.join
     m1.locked?.should be_false
   end
+
+  it "blocks the caller if already locked" do
+    m = Mutex.new
+    m.lock
+    lambda { m.synchronize { } }.should block_caller
+  end
+
+  it "does not block the caller if not locked" do
+    m = Mutex.new
+    lambda { m.synchronize { } }.should_not block_caller
+  end
+
+  it "blocks the caller if another thread is also in the synchronize block" do
+    m = Mutex.new
+    q1 = Queue.new
+    q2 = Queue.new
+
+    t = Thread.new {
+      m.synchronize {
+        q1.push :ready
+        q2.pop
+      }
+    }
+
+    q1.pop.should == :ready
+
+    lambda { m.synchronize { } }.should block_caller
+
+    q2.push :done
+    t.join
+  end
+
+  it "is not recursive" do
+    m = Mutex.new
+
+    m.synchronize do
+      lambda { m.synchronize { } }.should raise_error(ThreadError)
+    end
+  end
 end

spec/ruby/core/queue/append_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/enque'
+
+describe "Queue#<<" do
+  it_behaves_like :queue_enq, :<<, -> { Queue.new }
+end

spec/ruby/core/queue/clear_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/clear'
+
+describe "Queue#clear" do
+  it_behaves_like :queue_clear, :clear, -> { Queue.new }
+end

spec/ruby/core/queue/close_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/close'
+
+describe "Queue#close" do
+  it_behaves_like :queue_close, :close, -> { Queue.new }
+end

spec/ruby/core/queue/closed_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/closed'
+
+describe "Queue#closed?" do
+  it_behaves_like :queue_closed?, :closed?, -> { Queue.new }
+end

spec/ruby/core/queue/deq_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/deque'
+
+describe "Queue#deq" do
+  it_behaves_like :queue_deq, :deq, -> { Queue.new }
+end

spec/ruby/core/queue/empty_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/empty'
+
+describe "Queue#empty?" do
+  it_behaves_like :queue_empty?, :empty?, -> { Queue.new }
+end

spec/ruby/core/queue/enq_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/enque'
+
+describe "Queue#enq" do
+  it_behaves_like :queue_enq, :enq, -> { Queue.new }
+end

spec/ruby/core/queue/length_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/length'
+
+describe "Queue#length" do
+  it_behaves_like :queue_length, :length, -> { Queue.new }
+end

spec/ruby/core/queue/num_waiting_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/num_waiting'
+
+describe "Queue#num_waiting" do
+  it_behaves_like :queue_num_waiting, :num_waiting, -> { Queue.new }
+end

spec/ruby/core/queue/pop_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/deque'
+
+describe "Queue#pop" do
+  it_behaves_like :queue_deq, :pop, -> { Queue.new }
+end

spec/ruby/core/queue/push_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/enque'
+
+describe "Queue#push" do
+  it_behaves_like :queue_enq, :push, -> { Queue.new }
+end

spec/ruby/core/queue/shift_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/deque'
+
+describe "Queue#shift" do
+  it_behaves_like :queue_deq, :shift, -> { Queue.new }
+end

spec/ruby/core/queue/size_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/length'
+
+describe "Queue#size" do
+  it_behaves_like :queue_length, :size, -> { Queue.new }
+end

spec/ruby/core/sizedqueue/append_spec.rb

@@ -0,0 +1,11 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/enque'
+require_relative '../../shared/sizedqueue/enque'
+
+describe "SizedQueue#<<" do
+  it_behaves_like :queue_enq, :<<, -> { SizedQueue.new(10) }
+end
+
+describe "SizedQueue#<<" do
+  it_behaves_like :sizedqueue_enq, :<<, ->(n) { SizedQueue.new(n) }
+end

spec/ruby/core/sizedqueue/clear_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/clear'
+
+describe "SizedQueue#clear" do
+  it_behaves_like :queue_clear, :clear, -> { SizedQueue.new(10) }
+end

spec/ruby/library/thread/sizedqueue/close_spec.rb → spec/ruby/core/sizedqueue/close_spec.rb

@@ -1,6 +1,5 @@
-require_relative '../../../spec_helper'
-require 'thread'
-require_relative '../shared/queue/close'
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/close'
 
 describe "SizedQueue#close" do
   it_behaves_like :queue_close, :close, -> { SizedQueue.new(10) }

spec/ruby/library/thread/sizedqueue/closed_spec.rb → spec/ruby/core/sizedqueue/closed_spec.rb

@@ -1,6 +1,5 @@
-require_relative '../../../spec_helper'
-require 'thread'
-require_relative '../shared/queue/closed'
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/closed'
 
 describe "SizedQueue#closed?" do
   it_behaves_like :queue_closed?, :closed?, -> { SizedQueue.new(10) }

spec/ruby/core/sizedqueue/deq_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/deque'
+
+describe "SizedQueue#deq" do
+  it_behaves_like :queue_deq, :deq, -> { SizedQueue.new(10) }
+end

spec/ruby/core/sizedqueue/empty_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/empty'
+
+describe "SizedQueue#empty?" do
+  it_behaves_like :queue_empty?, :empty?, -> { SizedQueue.new(10) }
+end

spec/ruby/core/sizedqueue/enq_spec.rb

@@ -0,0 +1,11 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/enque'
+require_relative '../../shared/sizedqueue/enque'
+
+describe "SizedQueue#enq" do
+  it_behaves_like :queue_enq, :enq, -> { SizedQueue.new(10) }
+end
+
+describe "SizedQueue#enq" do
+  it_behaves_like :sizedqueue_enq, :enq, ->(n) { SizedQueue.new(n) }
+end

spec/ruby/core/sizedqueue/length_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/length'
+
+describe "SizedQueue#length" do
+  it_behaves_like :queue_length, :length, -> { SizedQueue.new(10) }
+end

spec/ruby/core/sizedqueue/max_spec.rb

@@ -0,0 +1,10 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/sizedqueue/max'
+
+describe "SizedQueue#max" do
+  it_behaves_like :sizedqueue_max, :max, ->(n) { SizedQueue.new(n) }
+end
+
+describe "SizedQueue#max=" do
+  it_behaves_like :sizedqueue_max=, :max=, ->(n) { SizedQueue.new(n) }
+end

spec/ruby/core/sizedqueue/new_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/sizedqueue/new'
+
+describe "SizedQueue.new" do
+  it_behaves_like :sizedqueue_new, :new, ->(*n) { SizedQueue.new(*n) }
+end

spec/ruby/core/sizedqueue/num_waiting_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/sizedqueue/num_waiting'
+
+describe "SizedQueue#num_waiting" do
+  it_behaves_like :sizedqueue_num_waiting, :new, ->(n) { SizedQueue.new(n) }
+end

spec/ruby/core/sizedqueue/pop_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/deque'
+
+describe "SizedQueue#pop" do
+  it_behaves_like :queue_deq, :pop, -> { SizedQueue.new(10) }
+end

spec/ruby/core/sizedqueue/push_spec.rb

@@ -0,0 +1,11 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/enque'
+require_relative '../../shared/sizedqueue/enque'
+
+describe "SizedQueue#push" do
+  it_behaves_like :queue_enq, :push, -> { SizedQueue.new(10) }
+end
+
+describe "SizedQueue#push" do
+  it_behaves_like :sizedqueue_enq, :push, ->(n) { SizedQueue.new(n) }
+end

spec/ruby/core/sizedqueue/shift_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/deque'
+
+describe "SizedQueue#shift" do
+  it_behaves_like :queue_deq, :shift, -> { SizedQueue.new(10) }
+end

spec/ruby/core/sizedqueue/size_spec.rb

@@ -0,0 +1,6 @@
+require_relative '../../spec_helper'
+require_relative '../../shared/queue/length'
+
+describe "SizedQueue#size" do
+  it_behaves_like :queue_length, :size, -> { SizedQueue.new(10) }
+end

spec/ruby/core/thread/exclusive_spec.rb

@@ -14,5 +14,29 @@
     Thread.exclusive { :result }.should == :result
   end
 
-  it "needs to be reviewed for spec completeness"
+  it "blocks the caller if another thread is also in an exclusive block" do
+    m = Mutex.new
+    q1 = Queue.new
+    q2 = Queue.new
+
+    t = Thread.new {
+      Thread.exclusive {
+        q1.push :ready
+        q2.pop
+      }
+    }
+
+    q1.pop.should == :ready
+
+    lambda { Thread.exclusive { } }.should block_caller
+
+    q2.push :done
+    t.join
+  end
+
+  it "is not recursive" do
+    Thread.exclusive do
+      lambda { Thread.exclusive { } }.should raise_error(ThreadError)
+    end
+  end
 end