Expose Semaphore::MAX_PERMITS and document it #5144

vi · 2022-10-29T15:23:26Z

Resolves #5129

tokio/src/sync/semaphore.rs

Darksonn · 2022-10-29T20:12:37Z

tokio/src/sync/semaphore.rs

-    /// The maximum number of permits is `usize::MAX >> 3`, and this function will panic if the limit is exceeded.
+    /// The maximum number of permits is [`Semaphore::MAX_PERMITS`], and this function will panic if the limit is exceeded.


Would be nice to keep the note that the value is usize::MAX >> 3 somewhere in the docs, though probably not here.

Does't specific number visible in the docs (it looks like this: pub const MAX_PERMITS: usize = 2_305_843_009_213_693_951usize) clarify this?

Or do you mean the documentation should state that this number is fixed and is not expected to change?

No, I mean that the docs should mention that 2_305_843_009_213_693_951usize is equal to usize::MAX >> 3.

Added sentence about that.

Also searched for other usize::MAX >> 3 in the docs and replaced with a link to MAX_PERMITS.

Another thing it would make sense to point out is that 2_305_843_009_213_693_951usize is only the correct number on 64-bit platforms.

usize::MAX >> 3 implies different value on different platforms.

tokio/src/sync/semaphore.rs

Darksonn · 2022-10-30T09:03:08Z

tokio/src/sync/semaphore.rs

+#[test]
+#[cfg(not(loom))]
+fn no_panic_at_maxpermits() {
+    let _ = Semaphore::new(Semaphore::MAX_PERMITS);
+    let s = Semaphore::new(Semaphore::MAX_PERMITS - 1);
+    s.add_permits(1);
+}
+
+#[test]
+#[cfg(not(loom))]
+#[should_panic]
+fn panic_when_exceeds_maxpermits1() {
+    let _ = Semaphore::new(Semaphore::MAX_PERMITS + 1);
+}
+#[test]
+#[cfg(not(loom))]
+#[should_panic]
+fn panic_when_exceeds_maxpermits2() {
+    let s = Semaphore::new(Semaphore::MAX_PERMITS - 1);
+    s.add_permits(1);
+    s.add_permits(1);
+}


Tests go in tests/sync_semaphore.rs

Moved the tests, also adjusted similar pre-existing tests.

Add and adjust some tests. Adjust related documentation. Resolves tokio-rs#5129

Darksonn

Thanks.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dependencies | minor | `1.21.2` -> `1.22.0` | | [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.21.2` -> `1.22.0` | --- ### Release Notes <details> <summary>tokio-rs/tokio</summary> ### [`v1.22.0`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.22.0): Tokio v1.22.0 [Compare Source](tokio-rs/tokio@tokio-1.21.2...tokio-1.22.0) ##### Added - runtime: add `Handle::runtime_flavor` ([#5138]) - sync: add `Mutex::blocking_lock_owned` ([#5130]) - sync: add `Semaphore::MAX_PERMITS` ([#5144]) - sync: add `merge()` to semaphore permits ([#4948]) - sync: add `mpsc::WeakUnboundedSender` ([#5189]) ##### Added (unstable) - process: add `Command::process_group` ([#5114]) - runtime: export metrics about the blocking thread pool ([#5161]) - task: add `task::id()` and `task::try_id()` ([#5171]) ##### Fixed - macros: don't take ownership of futures in macros ([#5087]) - runtime: fix Stacked Borrows violation in `LocalOwnedTasks` ([#5099]) - runtime: mitigate ABA with 32-bit queue indices when possible ([#5042]) - task: wake local tasks to the local queue when woken by the same thread ([#5095]) - time: panic in release mode when `mark_pending` called illegally ([#5093]) - runtime: fix typo in expect message ([#5169]) - runtime: fix `unsync_load` on atomic types ([#5175]) - task: elaborate safety comments in task deallocation ([#5172]) - runtime: fix `LocalSet` drop in thread local ([#5179]) - net: remove libc type leakage in a public API ([#5191]) - runtime: update the alignment of `CachePadded` ([#5106]) ##### Changed - io: make `tokio::io::copy` continue filling the buffer when writer stalls ([#5066]) - runtime: remove `coop::budget` from `LocalSet::run_until` ([#5155]) - sync: make `Notify` panic safe ([#5154]) ##### Documented - io: fix doc for `write_i8` to use signed integers ([#5040]) - net: fix doc typos for TCP and UDP `set_tos` methods ([#5073]) - net: fix function name in `UdpSocket::recv` documentation ([#5150]) - sync: typo in `TryLockError` for `RwLock::try_write` ([#5160]) - task: document that spawned tasks execute immediately ([#5117]) - time: document return type of `timeout` ([#5118]) - time: document that `timeout` checks only before poll ([#5126]) - sync: specify return type of `oneshot::Receiver` in docs ([#5198]) ##### Internal changes - runtime: use const `Mutex::new` for globals ([#5061]) - runtime: remove `Option` around `mio::Events` in io driver ([#5078]) - runtime: remove a conditional compilation clause ([#5104]) - runtime: remove a reference to internal time handle ([#5107]) - runtime: misc time driver cleanup ([#5120]) - runtime: move signal driver to runtime module ([#5121]) - runtime: signal driver now uses I/O driver directly ([#5125]) - runtime: start decoupling I/O driver and I/O handle ([#5127]) - runtime: switch `io::handle` refs with scheduler:Handle ([#5128]) - runtime: remove Arc from I/O driver ([#5134]) - runtime: use signal driver handle via `scheduler::Handle` ([#5135]) - runtime: move internal clock fns out of context ([#5139]) - runtime: remove `runtime::context` module ([#5140]) - runtime: keep driver cfgs in `driver.rs` ([#5141]) - runtime: add `runtime::context` to unify thread-locals ([#5143]) - runtime: rename some confusing internal variables/fns ([#5151]) - runtime: move `coop` mod into `runtime` ([#5152]) - runtime: move budget state to context thread-local ([#5157]) - runtime: move park logic into runtime module ([#5158]) - runtime: move `Runtime` into its own file ([#5159]) - runtime: unify entering a runtime with `Handle::enter` ([#5163]) - runtime: remove handle reference from each scheduler ([#5166]) - runtime: move `enter` into `context` ([#5167]) - runtime: combine context and entered thread-locals ([#5168]) - runtime: fix accidental unsetting of current handle ([#5178]) - runtime: move `CoreStage` methods to `Core` ([#5182]) - sync: name mpsc semaphore types ([#5146]) [#4948]: tokio-rs/tokio#4948 [#5040]: tokio-rs/tokio#5040 [#5042]: tokio-rs/tokio#5042 [#5061]: tokio-rs/tokio#5061 [#5066]: tokio-rs/tokio#5066 [#5073]: tokio-rs/tokio#5073 [#5078]: tokio-rs/tokio#5078 [#5087]: tokio-rs/tokio#5087 [#5093]: tokio-rs/tokio#5093 [#5095]: tokio-rs/tokio#5095 [#5099]: tokio-rs/tokio#5099 [#5104]: tokio-rs/tokio#5104 [#5106]: tokio-rs/tokio#5106 [#5107]: tokio-rs/tokio#5107 [#5114]: tokio-rs/tokio#5114 [#5117]: tokio-rs/tokio#5117 [#5118]: tokio-rs/tokio#5118 [#5120]: tokio-rs/tokio#5120 [#5121]: tokio-rs/tokio#5121 [#5125]: tokio-rs/tokio#5125 [#5126]: tokio-rs/tokio#5126 [#5127]: tokio-rs/tokio#5127 [#5128]: tokio-rs/tokio#5128 [#5130]: tokio-rs/tokio#5130 [#5134]: tokio-rs/tokio#5134 [#5135]: tokio-rs/tokio#5135 [#5138]: tokio-rs/tokio#5138 [#5138]: tokio-rs/tokio#5138 [#5139]: tokio-rs/tokio#5139 [#5140]: tokio-rs/tokio#5140 [#5141]: tokio-rs/tokio#5141 [#5143]: tokio-rs/tokio#5143 [#5144]: tokio-rs/tokio#5144 [#5144]: tokio-rs/tokio#5144 [#5146]: tokio-rs/tokio#5146 [#5150]: tokio-rs/tokio#5150 [#5151]: tokio-rs/tokio#5151 [#5152]: tokio-rs/tokio#5152 [#5154]: tokio-rs/tokio#5154 [#5155]: tokio-rs/tokio#5155 [#5157]: tokio-rs/tokio#5157 [#5158]: tokio-rs/tokio#5158 [#5159]: tokio-rs/tokio#5159 [#5160]: tokio-rs/tokio#5160 [#5161]: tokio-rs/tokio#5161 [#5163]: tokio-rs/tokio#5163 [#5166]: tokio-rs/tokio#5166 [#5167]: tokio-rs/tokio#5167 [#5168]: tokio-rs/tokio#5168 [#5169]: tokio-rs/tokio#5169 [#5171]: tokio-rs/tokio#5171 [#5172]: tokio-rs/tokio#5172 [#5175]: tokio-rs/tokio#5175 [#5178]: tokio-rs/tokio#5178 [#5179]: tokio-rs/tokio#5179 [#5182]: tokio-rs/tokio#5182 [#5189]: tokio-rs/tokio#5189 [#5191]: tokio-rs/tokio#5191 [#5198]: tokio-rs/tokio#5198 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1651 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

vi force-pushed the expose_MAX_PERMITS branch from 475384d to b098029 Compare October 29, 2022 15:27

github-actions bot added the R-loom Run loom tests on this PR label Oct 29, 2022

Darksonn added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Oct 29, 2022

Darksonn reviewed Oct 29, 2022

View reviewed changes

tokio/src/sync/semaphore.rs Outdated Show resolved Hide resolved

vi force-pushed the expose_MAX_PERMITS branch from b098029 to e648b6e Compare October 29, 2022 16:58

Darksonn reviewed Oct 29, 2022

View reviewed changes

Darksonn reviewed Oct 30, 2022

View reviewed changes

sync: Expose Semaphore::MAX_PERMITS as public API

57cc65e

Add and adjust some tests. Adjust related documentation. Resolves tokio-rs#5129

vi force-pushed the expose_MAX_PERMITS branch 3 times, most recently from c8a3006 to 57cc65e Compare October 30, 2022 20:14

Darksonn approved these changes Oct 30, 2022

View reviewed changes

Darksonn enabled auto-merge (squash) October 30, 2022 20:52

Darksonn merged commit d1a8ec6 into tokio-rs:master Oct 30, 2022

Darksonn mentioned this pull request Nov 17, 2022

chore: prepare Tokio v1.22.0 #5203

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose Semaphore::MAX_PERMITS and document it #5144

Expose Semaphore::MAX_PERMITS and document it #5144

vi commented Oct 29, 2022

Darksonn Oct 29, 2022

vi Oct 29, 2022 •

edited

Darksonn Oct 30, 2022

vi Oct 30, 2022

Darksonn Oct 30, 2022

vi Oct 30, 2022

Darksonn Oct 30, 2022

vi Oct 30, 2022

Darksonn left a comment

		/// The maximum number of permits is `usize::MAX >> 3`, and this function will panic if the limit is exceeded.
		/// The maximum number of permits is [`Semaphore::MAX_PERMITS`], and this function will panic if the limit is exceeded.

Expose Semaphore::MAX_PERMITS and document it #5144

Expose Semaphore::MAX_PERMITS and document it #5144

Conversation

vi commented Oct 29, 2022

Darksonn Oct 29, 2022

Choose a reason for hiding this comment

vi Oct 29, 2022 • edited

Choose a reason for hiding this comment

Darksonn Oct 30, 2022

Choose a reason for hiding this comment

vi Oct 30, 2022

Choose a reason for hiding this comment

Darksonn Oct 30, 2022

Choose a reason for hiding this comment

vi Oct 30, 2022

Choose a reason for hiding this comment

Darksonn Oct 30, 2022

Choose a reason for hiding this comment

vi Oct 30, 2022

Choose a reason for hiding this comment

Darksonn left a comment

Choose a reason for hiding this comment

vi Oct 29, 2022 •

edited