Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose semaphore's MAX_PERMITS as a public constant. #5129

Closed
vi opened this issue Oct 26, 2022 · 1 comment · Fixed by #5144
Closed

Expose semaphore's MAX_PERMITS as a public constant. #5129

vi opened this issue Oct 26, 2022 · 1 comment · Fixed by #5144
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-sync Module: tokio/sync

Comments

@vi
Copy link
Contributor

vi commented Oct 26, 2022

Is your feature request related to a problem? Please describe.

Sometimes semaphore needs to be disabled at runtime by adjusting it's configuration. But obvious usize::MAX fails with a semaphore may not have more than MAX_PERMITS permits (2305843009213693951) panic.

But MAX_PERMITS mentioned in the panic message is not available in API. Apart from serving as "effective infinity", it may be used to filter user input, preventing causing panics by setting deliberately invalid value. How are apps supposed to check user-configurable concurrency limit without panicking?

Describe the solution you'd like

pub const tokio::sync::Semaphore::MAX_PERMITS: usize = ...;

Describe alternatives you've considered

  • A fallible function for creating a semaphore, with Tower handling those errors.
  • Documenting 2305843009213693951 explicitly as a limit and never shrinking it, so apps can safely embed it.
  • Documenting that semaphores should not be operated close to the cap, if there are indeed valid reasons for that.
@vi vi added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Oct 26, 2022
@vi vi changed the title Expose semaphore's MAX_PERMITS permits as a public constant. Expose semaphore's MAX_PERMITS as a public constant. Oct 26, 2022
@Darksonn Darksonn added the M-sync Module: tokio/sync label Oct 29, 2022
@Darksonn
Copy link
Contributor

This seems reasonable enough.

vi added a commit to vi/tokio that referenced this issue Oct 29, 2022
vi added a commit to vi/tokio that referenced this issue Oct 29, 2022
vi added a commit to vi/tokio that referenced this issue Oct 29, 2022
vi added a commit to vi/tokio that referenced this issue Oct 30, 2022
Add and adjust some tests.
Adjust related documentation.

Resolves tokio-rs#5129
vi added a commit to vi/tokio that referenced this issue Oct 30, 2022
Add and adjust some tests.
Adjust related documentation.

Resolves tokio-rs#5129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants