Skip to content
This repository has been archived by the owner on Apr 2, 2018. It is now read-only.

Initial Capacity = Max(Initial Capacity, Channel Capacity) #29

Open
GGist opened this issue Jul 7, 2017 · 1 comment
Open

Initial Capacity = Max(Initial Capacity, Channel Capacity) #29

GGist opened this issue Jul 7, 2017 · 1 comment

Comments

@GGist
Copy link

GGist commented Jul 7, 2017

I am seeing issues because of this. For example, the following code:

    timer::wheel()
        .num_slots(8)
        .tick_duration(Duration::from_millis(58))
        .max_capacity(2000)
        .channel_capacity(2000)
        .num_slots(2048)
        .build();

Will cause problems at the call to unwrap at https://github.com/tokio-rs/tokio-timer/blob/master/src/worker.rs#L56 since https://github.com/tokio-rs/tokio-timer/blob/master/src/worker.rs#L52 won't protect us if capacity is not a power of two, since we will end up increasing capacity at https://github.com/tokio-rs/tokio-timer/blob/master/src/mpmc.rs#L32 which then causes the outer call to unwrap to fail.

This code where channel capacity is greater than max capacity also causes a different issue:

    timer::wheel()
        .num_slots(8)
        .tick_duration(Duration::from_millis(58))
        .max_capacity(2000)
        .channel_capacity(2001)
        .num_slots(2048)
        .build();

The error occurs at https://github.com/tokio-rs/tokio-timer/blob/master/src/wheel.rs#L109 due to a subtraction underflow (because by the time https://github.com/tokio-rs/tokio-timer/blob/master/src/wheel.rs#L107 is triggered, we were already past self.max_capacity since initial capacity was derived from a channel capacity that was greater than max capacity).

I don't mind submitting a PR, but wanted to check first what would be the best course of action, as there are a couple ways to fix these two issues.

@GGist
Copy link
Author

GGist commented Jul 7, 2017

In particular, I was thinking we could do two things in the builder:

  • Do the power of two promotion on channel capacity in the channel capacity getter
  • Compare initial capacity against max capacity in the initial capacity getter

Let me know what you think.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant