Skip to content

Commit 4a4728e

Browse files
committed
Fix JavaSemaphore not allowing #new and #reduce_permits with zero
These are now at feature-parity with the MutexSemaphore implementation.
1 parent f84c61f commit 4a4728e

File tree

2 files changed

+31
-9
lines changed

2 files changed

+31
-9
lines changed

ext/com/concurrent_ruby/ext/JavaSemaphoreLibrary.java

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,13 @@ public JavaSemaphore(Ruby runtime, RubyClass metaClass) {
4040

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

4747
@JRubyMethod
4848
public IRubyObject acquire(ThreadContext context, IRubyObject value) throws InterruptedException {
49-
this.semaphore.acquire(rubyFixnumToInt(value, "permits"));
49+
this.semaphore.acquire(rubyFixnumToPositiveInt(value, "permits"));
5050
return context.nil;
5151
}
5252

@@ -73,14 +73,14 @@ public IRubyObject tryAcquire(ThreadContext context) throws InterruptedException
7373

7474
@JRubyMethod(name = "try_acquire")
7575
public IRubyObject tryAcquire(ThreadContext context, IRubyObject permits) throws InterruptedException {
76-
return getRuntime().newBoolean(semaphore.tryAcquire(rubyFixnumToInt(permits, "permits")));
76+
return getRuntime().newBoolean(semaphore.tryAcquire(rubyFixnumToPositiveInt(permits, "permits")));
7777
}
7878

7979
@JRubyMethod(name = "try_acquire")
8080
public IRubyObject tryAcquire(ThreadContext context, IRubyObject permits, IRubyObject timeout) throws InterruptedException {
8181
return getRuntime().newBoolean(
8282
semaphore.tryAcquire(
83-
rubyFixnumToInt(permits, "permits"),
83+
rubyFixnumToPositiveInt(permits, "permits"),
8484
rubyNumericToLong(timeout, "timeout"),
8585
java.util.concurrent.TimeUnit.SECONDS)
8686
);
@@ -94,22 +94,31 @@ public IRubyObject release(ThreadContext context) {
9494

9595
@JRubyMethod
9696
public IRubyObject release(ThreadContext context, IRubyObject value) {
97-
this.semaphore.release(rubyFixnumToInt(value, "permits"));
97+
this.semaphore.release(rubyFixnumToPositiveInt(value, "permits"));
9898
return getRuntime().newBoolean(true);
9999
}
100100

101101
@JRubyMethod(name = "reduce_permits")
102102
public IRubyObject reducePermits(ThreadContext context, IRubyObject reduction) throws InterruptedException {
103-
this.semaphore.publicReducePermits(rubyFixnumToInt(reduction, "reduction"));
103+
this.semaphore.publicReducePermits(rubyFixnumToNonNegativeInt(reduction, "reduction"));
104104
return context.nil;
105105
}
106106

107-
private int rubyFixnumToInt(IRubyObject value, String paramName) {
107+
private int rubyFixnumToNonNegativeInt(IRubyObject value, String paramName) {
108+
if (value instanceof RubyFixnum && ((RubyFixnum) value).getLongValue() >= 0) {
109+
RubyFixnum fixNum = (RubyFixnum) value;
110+
return (int) fixNum.getLongValue();
111+
} else {
112+
throw getRuntime().newArgumentError(paramName + " must be a non-negative integer");
113+
}
114+
}
115+
116+
private int rubyFixnumToPositiveInt(IRubyObject value, String paramName) {
108117
if (value instanceof RubyFixnum && ((RubyFixnum) value).getLongValue() > 0) {
109118
RubyFixnum fixNum = (RubyFixnum) value;
110119
return (int) fixNum.getLongValue();
111120
} else {
112-
throw getRuntime().newArgumentError(paramName + " must be in integer greater than zero");
121+
throw getRuntime().newArgumentError(paramName + " must be an integer greater than zero");
113122
}
114123
}
115124

@@ -118,7 +127,7 @@ private long rubyNumericToLong(IRubyObject value, String paramName) {
118127
RubyNumeric fixNum = (RubyNumeric) value;
119128
return fixNum.getLongValue();
120129
} else {
121-
throw getRuntime().newArgumentError(paramName + " must be in float greater than zero");
130+
throw getRuntime().newArgumentError(paramName + " must be a float greater than zero");
122131
}
123132
}
124133

spec/concurrent/atomic/semaphore_spec.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@
77
described_class.new('foo')
88
}.to raise_error(ArgumentError)
99
end
10+
11+
context 'when initializing with 0' do
12+
let(:semaphore) { described_class.new(0) }
13+
14+
it do
15+
expect(semaphore).to_not be nil
16+
end
17+
end
1018
end
1119

1220
describe '#acquire' do
@@ -111,6 +119,11 @@
111119
semaphore.reduce_permits 2
112120
expect(semaphore.available_permits).to eq 0
113121
end
122+
123+
it 'reduces zero permits' do
124+
semaphore.reduce_permits 0
125+
expect(semaphore.available_permits).to eq 3
126+
end
114127
end
115128

116129
describe '#release' do

0 commit comments

Comments
 (0)