Skip to content

Commit

Permalink
sync: Fail with init semaphore count < 0
Browse files Browse the repository at this point in the history
Semaphores are not currently designed to handle this case correctly, leading to
very strange behavior. Semaphores as written are intended to count *resources*
and it's not possible to have a negative number of resources.

This alters the behavior and documentation to note that the task will be failed
if the initial count is 0.

Closes #15758
  • Loading branch information
alexcrichton committed Jul 18, 2014
1 parent 4418664 commit 3419e20
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions src/libsync/raw.rs
Expand Up @@ -109,6 +109,8 @@ struct SemGuard<'a, Q> {

impl<Q: Send> Sem<Q> {
fn new(count: int, q: Q) -> Sem<Q> {
assert!(count >= 0,
"semaphores cannot be initialized with negative values");
Sem {
lock: mutex::Mutex::new(),
inner: Unsafe::new(SemInner {
Expand Down Expand Up @@ -364,6 +366,10 @@ pub struct SemaphoreGuard<'a> {

impl Semaphore {
/// Create a new semaphore with the specified count.
///
/// # Failure
///
/// This function will fail if `count` is negative.
pub fn new(count: int) -> Semaphore {
Semaphore { sem: Sem::new(count, ()) }
}
Expand Down Expand Up @@ -637,6 +643,11 @@ mod tests {
let _g = s.access();
}
#[test]
#[should_fail]
fn test_sem_basic2() {
Semaphore::new(-1);
}
#[test]
fn test_sem_as_mutex() {
let s = Arc::new(Semaphore::new(1));
let s2 = s.clone();
Expand Down

5 comments on commit 3419e20

@bors
Copy link
Contributor

@bors bors commented on 3419e20 Jul 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from bblum
at alexcrichton@3419e20

@bors
Copy link
Contributor

@bors bors commented on 3419e20 Jul 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging alexcrichton/rust/issue-15758 = 3419e20 into auto

@bors
Copy link
Contributor

@bors bors commented on 3419e20 Jul 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexcrichton/rust/issue-15758 = 3419e20 merged ok, testing candidate = 221c28a

@bors
Copy link
Contributor

@bors bors commented on 3419e20 Jul 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 221c28a

Please sign in to comment.