Refactored tests for Condition and Exchanger.

Author: jdantonio

spec/concurrent/atomic/condition_spec.rb

@@ -2,15 +2,6 @@ module Concurrent
 
   describe Condition do
 
-    let(:mutex) { Mutex.new }
-    subject{ Condition.new }
-
-    before(:each) do
-      # rspec is not thread safe, without mutex initialization
-      # we can experience race conditions
-      mutex
-    end
-
     context 'with no waiting threads' do
       describe '#signal' do
         it 'should return immediately' do
@@ -32,25 +23,47 @@ module Concurrent
         describe '#wait without timeout' do
 
           it 'should block the thread' do
-            latch = Concurrent::CountDownLatch.new
+            latch_1 = Concurrent::CountDownLatch.new
+            latch_2 = Concurrent::CountDownLatch.new
+            mutex = Mutex.new
+
             t = Thread.new do
               mutex.synchronize do
-                latch.count_down
+                latch_1.count_down
                 subject.wait(mutex)
+                latch_2.count_down
               end
             end
 
-            latch.wait(1)
+            latch_1.wait(1)
+            latch_2.wait(0.1)
             expect(t.status).to eq 'sleep'
+            expect(latch_2.count).to eq 1
             t.kill
           end
 
           it 'should return a woken up result when is woken up by #signal' do
             result = nil
-            t = Thread.new { mutex.synchronize { result = subject.wait(mutex) } }
-            sleep(0.1)
-            mutex.synchronize { subject.signal }
-            sleep(0.1)
+            mutex = Mutex.new
+            latch_1 = Concurrent::CountDownLatch.new
+            latch_2 = Concurrent::CountDownLatch.new
+
+            t = Thread.new do
+              mutex.synchronize do
+                latch_1.count_down
+                result = subject.wait(mutex)
+                latch_2.count_down
+              end
+            end
+
+            latch_1.wait(1)
+
+            mutex.synchronize do
+              subject.signal
+            end
+
+            latch_2.wait(1)
+
             expect(result).to be_woken_up
             expect(result).not_to be_timed_out
             expect(result.remaining_time).to be_nil
@@ -59,72 +72,136 @@ module Concurrent
 
           it 'should return a woken up result when is woken up by #broadcast' do
             result = nil
-            t = Thread.new { mutex.synchronize { result = subject.wait(mutex) } }
-            sleep(0.1)
-            mutex.synchronize { subject.broadcast }
-            sleep(0.1)
+            mutex = Mutex.new
+            latch_1 = Concurrent::CountDownLatch.new
+            latch_2 = Concurrent::CountDownLatch.new
+
+            t = Thread.new do
+              mutex.synchronize do
+                latch_1.count_down
+                result = subject.wait(mutex)
+                latch_2.count_down
+              end
+            end
+
+            latch_1.wait(1)
+
+            mutex.synchronize do
+              subject.broadcast
+            end
+
+            latch_2.wait(1)
+
             expect(result).to be_woken_up
             expect(result).not_to be_timed_out
             expect(result.remaining_time).to be_nil
             expect(t.status).to be_falsey
           end
         end
-
       end
 
       context 'timeout' do
 
         describe '#wait' do
 
           it 'should block the thread' do
-            latch = Concurrent::CountDownLatch.new
+            latch_1 = Concurrent::CountDownLatch.new
+            latch_2 = Concurrent::CountDownLatch.new
+            mutex = Mutex.new
+
             t = Thread.new do
               mutex.synchronize do
-                latch.count_down
+                latch_1.count_down
                 subject.wait(mutex, 1)
+                latch_2.count_down
               end
             end
 
-            latch.wait(1)
+            latch_1.wait(1)
+            latch_2.wait(0.1)
             expect(t.status).to eq 'sleep'
+            expect(latch_2.count).to eq 1
             t.kill
           end
 
           it 'should return remaining time when is woken up by #signal' do
             result = nil
-            t = Thread.new { mutex.synchronize { result = subject.wait(mutex, 1) } }
-            sleep(0.1)
-            mutex.synchronize { subject.signal }
-            sleep(0.1)
+            mutex = Mutex.new
+            latch_1 = Concurrent::CountDownLatch.new
+            latch_2 = Concurrent::CountDownLatch.new
+
+            t = Thread.new do
+              mutex.synchronize do
+                latch_1.count_down
+                result = subject.wait(mutex, 1)
+                latch_2.count_down
+              end
+            end
+
+            latch_1.wait(1)
+
+            mutex.synchronize do
+              sleep(0.1)
+              subject.signal
+            end
+
+            latch_2.wait(1)
+
             expect(result).to be_woken_up
             expect(result).not_to be_timed_out
-            expect(result.remaining_time).to be_within(0.1).of(0.85)
+            expect(result.remaining_time).to be < 1.0
             expect(t.status).to be_falsey
           end
 
           it 'should return remaining time when is woken up by #broadcast' do
             result = nil
-            t = Thread.new { mutex.synchronize { result = subject.wait(mutex, 1) } }
-            sleep(0.1)
-            mutex.synchronize { subject.broadcast }
-            sleep(0.1)
+            mutex = Mutex.new
+            latch_1 = Concurrent::CountDownLatch.new
+            latch_2 = Concurrent::CountDownLatch.new
+
+            t = Thread.new do
+              mutex.synchronize do
+                latch_1.count_down
+                result = subject.wait(mutex, 1)
+                latch_2.count_down
+              end
+            end
+
+            latch_1.wait(1)
+
+            mutex.synchronize do
+              sleep(0.1)
+              subject.broadcast
+            end
+
+            latch_2.wait(1)
+
             expect(result).to be_woken_up
             expect(result).not_to be_timed_out
-            expect(result.remaining_time).to be_within(0.1).of(0.85)
+            expect(result.remaining_time).to be < 1.0
             expect(t.status).to be_falsey
           end
 
           it 'should return 0 or negative number if timed out' do
             result = nil
-            t = Thread.new { mutex.synchronize { result = subject.wait(mutex, 0.1) } }
-            sleep(0.2)
+            mutex = Mutex.new
+            latch = Concurrent::CountDownLatch.new
+
+            t = Thread.new do
+              mutex.synchronize do
+                result = subject.wait(mutex, 0.1)
+                latch.count_down
+              end
+            end
+
+            latch.wait(1)
+
             expect(result).not_to be_woken_up
             expect(result).to be_timed_out
             expect(result.remaining_time).to be_less_than_or_equal_to(0)
             expect(t.status).to be_falsey
           end
         end
-
       end
     end
 
@@ -135,34 +212,53 @@ module Concurrent
         describe '#wait' do
 
           it 'should block threads' do
-            t1 = Thread.new { mutex.synchronize { subject.wait(mutex) } }
-            t2 = Thread.new { mutex.synchronize { subject.wait(mutex) } }
+            mutex = Mutex.new
+            latch = Concurrent::CountDownLatch.new(2)
+            t1 = Thread.new { mutex.synchronize { latch.count_down; subject.wait(mutex) } }
+            t2 = Thread.new { mutex.synchronize { latch.count_down; subject.wait(mutex) } }
+            latch.wait(1)
             sleep(0.1)
             [t1, t2].each { |t| expect(t.status).to eq 'sleep' }
             [t1, t2].each { |t| t.kill }
           end
-
         end
 
         describe '#signal' do
           it 'wakes up only one thread' do
-            latch = CountDownLatch.new(2)
+            latch_1 = Concurrent::CountDownLatch.new(2)
+            latch_2 = Concurrent::CountDownLatch.new(2)
+            mutex = Mutex.new
 
-            t1 = Thread.new { mutex.synchronize { subject.wait(mutex); latch.count_down } }
-            t2 = Thread.new { mutex.synchronize { subject.wait(mutex); latch.count_down } }
+            t1 = Thread.new do
+              mutex.synchronize do
+                latch_1.count_down
+                subject.wait(mutex)
+                latch_2.count_down
+              end
+            end
 
+            t2 = Thread.new do
+              mutex.synchronize do
+                latch_1.count_down
+                subject.wait(mutex)
+                latch_2.count_down
+              end
+            end
+
+            latch_1.wait(1)
             sleep(0.1)
             mutex.synchronize { subject.signal }
-            sleep(0.2)
+            sleep(0.1)
 
-            expect(latch.count).to eq 1
+            expect(latch_2.count).to eq 1
             [t1, t2].each { |t| t.kill }
           end
         end
 
         describe '#broadcast' do
           it 'wakes up all threads' do
             latch = CountDownLatch.new(2)
+            mutex = Mutex.new
 
             t1 = Thread.new { mutex.synchronize { subject.wait(mutex); latch.count_down } }
             t2 = Thread.new { mutex.synchronize { subject.wait(mutex); latch.count_down } }
@@ -176,8 +272,6 @@ module Concurrent
           end
         end
       end
-
     end
-
   end
 end

spec/concurrent/exchanger_spec.rb

@@ -4,72 +4,52 @@ module Concurrent
 
   describe Exchanger do
 
-    subject { Exchanger.new }
-    let!(:exchanger) { subject } # let is not thread safe, let! creates the object before ensuring uniqueness
-
     describe 'exchange' do
 
       context 'without timeout' do
 
         it 'should block' do
-          latch = Concurrent::CountDownLatch.new
+          latch_1 = Concurrent::CountDownLatch.new
+          latch_2 = Concurrent::CountDownLatch.new
 
           t = Thread.new do
-            latch.count_down
-            exchanger.exchange(1)
+            latch_1.count_down
+            subject.exchange(1)
+            latch_2.count_down
           end
 
-          latch.wait(1)
+          latch_1.wait(1)
+          latch_2.wait(0.1)
           expect(t.status).to eq 'sleep'
+          expect(latch_2.count).to eq 1
+          t.kill
         end
 
         it 'should receive the other value' do
-          latch = Concurrent::CountDownLatch.new(2)
           first_value = nil
           second_value = nil
 
-          Thread.new do
-            first_value = exchanger.exchange(2)
-            latch.count_down
-          end
-          Thread.new do
-            second_value = exchanger.exchange(4)
-            latch.count_down
-          end
+          thread_1 = Thread.new { first_value = subject.exchange(2) }
+          thread_2 = Thread.new { second_value = subject.exchange(4) }
 
-          latch.wait(1)
+          [thread_1, thread_2].each(&:join)
           expect(first_value).to eq 4
           expect(second_value).to eq 2
         end
 
         it 'can be reused' do
-          latch_1 = Concurrent::CountDownLatch.new(2)
-          latch_2 = Concurrent::CountDownLatch.new(2)
-
           first_value = nil
           second_value = nil
 
-          Thread.new do
-            first_value = exchanger.exchange(1)
-            latch_1.count_down
-          end
-          Thread.new do
-            second_value = exchanger.exchange(0)
-            latch_1.count_down
-          end
+          thread_1 = Thread.new { first_value = subject.exchange(1) }
+          thread_2 = Thread.new { second_value = subject.exchange(0) }
 
-          latch_1.wait(1)
+          [thread_1, thread_2].each(&:join)
 
-          Thread.new do
-            first_value = exchanger.exchange(10)
-            latch_2.count_down
-          end
-          Thread.new do
-            second_value = exchanger.exchange(12)
-            latch_2.count_down
-          end
+          thread_1 = Thread.new { first_value = subject.exchange(10) }
+          thread_2 = Thread.new { second_value = subject.exchange(12) }
 
-          latch_2.wait(1)
+          [thread_1, thread_2].each(&:join)
 
           expect(first_value).to eq 12
           expect(second_value).to eq 10
@@ -80,7 +60,7 @@ module Concurrent
 
         it 'should block until timeout' do
           duration = Hitimes::Interval.measure do
-            exchanger.exchange(2, 0.1)
+            subject.exchange(2, 0.1)
           end
           expect(duration).to be_within(0.05).of(0.1)
         end