Merge pull request #596 from pitr-ch/semaphore

Match Semaphore behaviors between Mutex and Java

Author: Petr Chalupa

ext/com/concurrent_ruby/ext/JavaSemaphoreLibrary.java

@@ -40,7 +40,7 @@ public JavaSemaphore(Ruby runtime, RubyClass metaClass) {
 
         @JRubyMethod
         public IRubyObject initialize(ThreadContext context, IRubyObject value) {
-            this.semaphore = new JRubySemaphore(rubyFixnumToNonNegativeInt(value, "count"));
+            this.semaphore = new JRubySemaphore(rubyFixnumInt(value, "count"));
             return context.nil;
         }
 
@@ -104,6 +104,15 @@ public IRubyObject reducePermits(ThreadContext context, IRubyObject reduction) t
             return context.nil;
         }
 
+        private int rubyFixnumInt(IRubyObject value, String paramName) {
+            if (value instanceof RubyFixnum) {
+                RubyFixnum fixNum = (RubyFixnum) value;
+                return (int) fixNum.getLongValue();
+            } else {
+                throw getRuntime().newArgumentError(paramName + " must be integer");
+            }
+        }
+
         private int rubyFixnumToNonNegativeInt(IRubyObject value, String paramName) {
             if (value instanceof RubyFixnum && ((RubyFixnum) value).getLongValue() >= 0) {
                 RubyFixnum fixNum = (RubyFixnum) value;

lib/concurrent/atomic/mutex_semaphore.rb

@@ -11,7 +11,6 @@ class MutexSemaphore < Synchronization::LockableObject
     # @!macro semaphore_method_initialize
     def initialize(count)
       Utility::NativeInteger.ensure_integer_and_bounds count
-      Utility::NativeInteger.ensure_positive count
 
       super()
       synchronize { ns_initialize count }
@@ -20,7 +19,7 @@ def initialize(count)
     # @!macro semaphore_method_acquire
     def acquire(permits = 1)
       Utility::NativeInteger.ensure_integer_and_bounds permits
-      Utility::NativeInteger.ensure_positive_and_no_zero permits
+      Utility::NativeInteger.ensure_positive permits
 
       synchronize do
         try_acquire_timed(permits, nil)
@@ -47,7 +46,7 @@ def drain_permits
     # @!macro semaphore_method_try_acquire
     def try_acquire(permits = 1, timeout = nil)
       Utility::NativeInteger.ensure_integer_and_bounds permits
-      Utility::NativeInteger.ensure_positive_and_no_zero permits
+      Utility::NativeInteger.ensure_positive permits
 
       synchronize do
         if timeout.nil?
@@ -61,7 +60,7 @@ def try_acquire(permits = 1, timeout = nil)
     # @!macro semaphore_method_release
     def release(permits = 1)
       Utility::NativeInteger.ensure_integer_and_bounds permits
-      Utility::NativeInteger.ensure_positive_and_no_zero permits
+      Utility::NativeInteger.ensure_positive permits
 
       synchronize do
         @free += permits

spec/concurrent/atomic/semaphore_spec.rb

@@ -15,6 +15,15 @@
         expect(semaphore).to_not be nil
       end
     end
+
+    context 'when initializing with -1' do
+      let(:semaphore) { described_class.new(-1) }
+
+      it do
+        semaphore.release
+        expect(semaphore.available_permits).to eq 0
+      end
+    end
   end
 
   describe '#acquire' do
@@ -36,10 +45,10 @@
       end
     end
 
-    context 'when acquiring zero permits' do
+    context 'when acquiring negative permits' do
       it do
         expect {
-          semaphore.acquire(0)
+          semaphore.acquire(-1)
         }.to raise_error(ArgumentError)
       end
     end
@@ -71,10 +80,10 @@
         expect(result).to be_falsey
       end
 
-      context 'when trying to acquire zero permits' do
+      context 'when trying to acquire negative permits' do
         it do
           expect {
-            semaphore.try_acquire(0)
+            semaphore.try_acquire(-1)
           }.to raise_error(ArgumentError)
         end
       end
@@ -138,10 +147,10 @@
         expect(semaphore.available_permits).to eq 5
       end
 
-      context 'when permits is set to zero' do
+      context 'when permits is set to negative number' do
         it do
           expect {
-            semaphore.release(0)
+            semaphore.release(-1)
           }.to raise_error(ArgumentError)
         end
       end