Skip to content

Commit 736ebd1

Browse files
committed
Match Semaphore behaviors between Mutex and Java
fixes #592
1 parent f297472 commit 736ebd1

File tree

3 files changed

+28
-11
lines changed

3 files changed

+28
-11
lines changed

ext/com/concurrent_ruby/ext/JavaSemaphoreLibrary.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public JavaSemaphore(Ruby runtime, RubyClass metaClass) {
4040

4141
@JRubyMethod
4242
public IRubyObject initialize(ThreadContext context, IRubyObject value) {
43-
this.semaphore = new JRubySemaphore(rubyFixnumToNonNegativeInt(value, "count"));
43+
this.semaphore = new JRubySemaphore(rubyFixnumInt(value, "count"));
4444
return context.nil;
4545
}
4646

@@ -104,6 +104,15 @@ public IRubyObject reducePermits(ThreadContext context, IRubyObject reduction) t
104104
return context.nil;
105105
}
106106

107+
private int rubyFixnumInt(IRubyObject value, String paramName) {
108+
if (value instanceof RubyFixnum) {
109+
RubyFixnum fixNum = (RubyFixnum) value;
110+
return (int) fixNum.getLongValue();
111+
} else {
112+
throw getRuntime().newArgumentError(paramName + " must be integer");
113+
}
114+
}
115+
107116
private int rubyFixnumToNonNegativeInt(IRubyObject value, String paramName) {
108117
if (value instanceof RubyFixnum && ((RubyFixnum) value).getLongValue() >= 0) {
109118
RubyFixnum fixNum = (RubyFixnum) value;

lib/concurrent/atomic/mutex_semaphore.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ class MutexSemaphore < Synchronization::LockableObject
1111
# @!macro semaphore_method_initialize
1212
def initialize(count)
1313
Utility::NativeInteger.ensure_integer_and_bounds count
14-
Utility::NativeInteger.ensure_positive count
1514

1615
super()
1716
synchronize { ns_initialize count }
@@ -20,7 +19,7 @@ def initialize(count)
2019
# @!macro semaphore_method_acquire
2120
def acquire(permits = 1)
2221
Utility::NativeInteger.ensure_integer_and_bounds permits
23-
Utility::NativeInteger.ensure_positive_and_no_zero permits
22+
Utility::NativeInteger.ensure_positive permits
2423

2524
synchronize do
2625
try_acquire_timed(permits, nil)
@@ -47,7 +46,7 @@ def drain_permits
4746
# @!macro semaphore_method_try_acquire
4847
def try_acquire(permits = 1, timeout = nil)
4948
Utility::NativeInteger.ensure_integer_and_bounds permits
50-
Utility::NativeInteger.ensure_positive_and_no_zero permits
49+
Utility::NativeInteger.ensure_positive permits
5150

5251
synchronize do
5352
if timeout.nil?
@@ -61,7 +60,7 @@ def try_acquire(permits = 1, timeout = nil)
6160
# @!macro semaphore_method_release
6261
def release(permits = 1)
6362
Utility::NativeInteger.ensure_integer_and_bounds permits
64-
Utility::NativeInteger.ensure_positive_and_no_zero permits
63+
Utility::NativeInteger.ensure_positive permits
6564

6665
synchronize do
6766
@free += permits

spec/concurrent/atomic/semaphore_spec.rb

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@
1515
expect(semaphore).to_not be nil
1616
end
1717
end
18+
19+
context 'when initializing with -1' do
20+
let(:semaphore) { described_class.new(-1) }
21+
22+
it do
23+
semaphore.release
24+
expect(semaphore.available_permits).to eq 0
25+
end
26+
end
1827
end
1928

2029
describe '#acquire' do
@@ -36,10 +45,10 @@
3645
end
3746
end
3847

39-
context 'when acquiring zero permits' do
48+
context 'when acquiring negative permits' do
4049
it do
4150
expect {
42-
semaphore.acquire(0)
51+
semaphore.acquire(-1)
4352
}.to raise_error(ArgumentError)
4453
end
4554
end
@@ -71,10 +80,10 @@
7180
expect(result).to be_falsey
7281
end
7382

74-
context 'when trying to acquire zero permits' do
83+
context 'when trying to acquire negative permits' do
7584
it do
7685
expect {
77-
semaphore.try_acquire(0)
86+
semaphore.try_acquire(-1)
7887
}.to raise_error(ArgumentError)
7988
end
8089
end
@@ -138,10 +147,10 @@
138147
expect(semaphore.available_permits).to eq 5
139148
end
140149

141-
context 'when permits is set to zero' do
150+
context 'when permits is set to negative number' do
142151
it do
143152
expect {
144-
semaphore.release(0)
153+
semaphore.release(-1)
145154
}.to raise_error(ArgumentError)
146155
end
147156
end

0 commit comments

Comments
 (0)