ruby-concurrency / concurrent-ruby

Modern concurrency tools including agents, futures, promises, thread pools, supervisors, and more. Inspired by Erlang, Clojure, Scala, Go, Java, JavaScript, and classic concurrency patterns.
https://ruby-concurrency.github.io/concurrent-ruby/
Other
5.68k stars 418 forks source link

Semaphore doesn't raise exception when negative int passed to constructor #967

Closed flash-gordon closed 1 year ago

flash-gordon commented 1 year ago

The docs say it'll raise an error but it doesn't:

Concurrent::Semaphore.new(-1)

I use cruby. There's no check for negative number in the code: https://github.dev/ruby-concurrency/concurrent-ruby/blob/385156117c110808875e1e87d5267147f696f796/lib/concurrent-ruby/concurrent/atomic/mutex_semaphore.rb#L12

eregon commented 1 year ago

Indeed, could you make a PR to add the check?

eregon commented 1 year ago

Looking more into this, it seems it's intended to allow a negative initial count since https://github.com/ruby-concurrency/concurrent-ruby/commit/736ebd16ff9570534527cf04c1af81ce7e2541bc, which also adds a spec for Concurrent::Semaphore.new(-1). So the docs are wrong and I'll update that. Semaphore can go negative, that seems a common feature of semaphores.

eregon commented 1 year ago

Docs are up to date now at https://ruby-concurrency.github.io/concurrent-ruby/master/Concurrent/Semaphore.html#initialize-instance_method