Add `set_max_level_racy` and gate `set_max_level` #544

djkoloski · 2023-01-06T16:14:56Z

Calling set_max_level can result in a race condition on platforms that don't have an atomic compare and swap implementation. This gates set_max_level behind #[cfg(atomic_cas)] and adds a racy alternative that can be called in these situations. This mirrors the approach for set_logger.

Fixes #534

Thomasdezeeuw · 2023-01-06T16:19:27Z

src/lib.rs

+/// [`set_max_level`]: fn.set_max_level.html
+#[inline]
+pub unsafe fn set_max_level_racy(level: LevelFilter) {
+    MAX_LOG_LEVEL_FILTER.store(level as usize, Ordering::Relaxed);


This is the same as the other function?

Yes, but the type of MAX_LOG_LEVEL_FILTER is different (core::sync::AtomicUsize vs log::AtomicUsize). When a platform doesn't support atomic_cas, the underlying implementation uses a Cell<usize> which can race with interrupts.

Ah, that makes sense. Then maybe a note pointing that out would be worth it?

I think that's a good idea, added a comment.

Calling `set_max_level` can result in a race condition on platforms that don't have an atomic compare and swap implementation. This gates `set_max_level` behind `#[cfg(atomic_cas)]` and adds a racy alternative that can be called in these situations. This mirrors the approach for `set_logger`.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [log](https://github.com/rust-lang/log) | dependencies | patch | `0.4.17` -> `0.4.19` | --- ### Release Notes <details> <summary>rust-lang/log</summary> ### [`v0.4.19`](https://github.com/rust-lang/log/blob/HEAD/CHANGELOG.md#0419---2023-06-10) [Compare Source](rust-lang/log@0.4.18...0.4.19) - Use target_has_atomic instead of the old atomic_cas cfg by [@GuillaumeGomez](https://github.com/GuillaumeGomez) in rust-lang/log#555 - Put MSRV into Cargo.toml by [@est31](https://github.com/est31) in rust-lang/log#557 ### [`v0.4.18`](https://github.com/rust-lang/log/blob/HEAD/CHANGELOG.md#0418---2023-05-28) [Compare Source](rust-lang/log@0.4.17...0.4.18) - fix markdown links (again) by [@hellow554](https://github.com/hellow554) in rust-lang/log#513 - add cargo doc to workflow by [@hellow554](https://github.com/hellow554) in rust-lang/log#515 - Apply Clippy lints by [@hellow554](https://github.com/hellow554) in rust-lang/log#516 - Replace ad-hoc eq_ignore_ascii_case with slice::eq_ignore_ascii_case by [@glandium](https://github.com/glandium) in rust-lang/log#519 - fix up windows targets by [@KodrAus](https://github.com/KodrAus) in rust-lang/log#528 - typo fix by [@jiangying000](https://github.com/jiangying000) in rust-lang/log#529 - Remove dependency on cfg_if by [@EriKWDev](https://github.com/EriKWDev) in rust-lang/log#536 - GitHub Workflows security hardening by [@sashashura](https://github.com/sashashura) in rust-lang/log#538 - Fix build status badge by [@atouchet](https://github.com/atouchet) in rust-lang/log#539 - Add call_logger to the documentation by [@a1ecbr0wn](https://github.com/a1ecbr0wn) in rust-lang/log#547 - Use stable internals for key-value API by [@KodrAus](https://github.com/KodrAus) in rust-lang/log#550 - Change wording of list of implementations by [@Thomasdezeeuw](https://github.com/Thomasdezeeuw) in rust-lang/log#553 - Add std-logger to list of implementations by [@Thomasdezeeuw](https://github.com/Thomasdezeeuw) in rust-lang/log#554 - Add `set_max_level_racy` and gate `set_max_level` by [@djkoloski](https://github.com/djkoloski) in rust-lang/log#544 - \[doc] src/lib.rs : prefix an unused variable with an underscore by [@OccupyMars2025](https://github.com/OccupyMars2025) in rust-lang/log#561 - \[doc] src/macros.rs : correct grammar errors of an example in lib documentation by [@OccupyMars2025](https://github.com/OccupyMars2025) in rust-lang/log#562 </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 this update 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 <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1911 Reviewed-by: crapStone <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>

* Optimization: Rm `debug!` in `find_version` printing all version iterated obviously doesn't help much in debugging in the problem but rather just confusing. Also this makes it hard for the compiler to optimize the iterators. * Use let-else in `ManifestVisitor` * Optimize `BinFile::preview_{bin, link}` for zero-copy Return `impl Display` that lazily format instead of allocating a `String` * Optimize `infer_bin_dir_template`: Generate dir lazily * Optimize `find_version`: Lazily clone `version_req` only on err * Refactor `find_version`: Use `bool::then_some` * Add dep either v1.8.0 to binstalk * Optimize `GhCrateMeta::find`: Avoid cloning and `Vec` creation by using `Either` * Optimize `ops::install::install_from_package`: Make it a regular fn instead of async fn since it does not `.await` on any async fn. * Optimize `QuickInstall`: Rm field `target` since `Arc<Data>` already contains that field. * Optimize `Fetcher`s: Extract new struct `TargetData` so that `Data` can be shared by all fetchers, regardless of the target. * Optimize `QuickInstall`: Rm unused field `data` * Optimize `Resolution::print`: Replace branching with conditional move Signed-off-by: Jiahao XU <[email protected]>

Thomasdezeeuw reviewed Jan 6, 2023

View reviewed changes

djkoloski force-pushed the add_set_max_level_racy branch from 4d17cdb to e5f1ae9 Compare January 6, 2023 16:28

Thomasdezeeuw approved these changes Jan 6, 2023

View reviewed changes

KodrAus merged commit 11eda98 into rust-lang:master Apr 13, 2023

KodrAus mentioned this pull request Apr 13, 2023

Use target_has_atomic instead of the old atomic_cas cfg #555

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `set_max_level_racy` and gate `set_max_level` #544

Add `set_max_level_racy` and gate `set_max_level` #544

djkoloski commented Jan 6, 2023 •

edited

Loading

Thomasdezeeuw Jan 6, 2023

djkoloski Jan 6, 2023

Thomasdezeeuw Jan 6, 2023

djkoloski Jan 6, 2023

Add set_max_level_racy and gate set_max_level #544

Add set_max_level_racy and gate set_max_level #544

Conversation

djkoloski commented Jan 6, 2023 • edited Loading

Thomasdezeeuw Jan 6, 2023

Choose a reason for hiding this comment

djkoloski Jan 6, 2023

Choose a reason for hiding this comment

Thomasdezeeuw Jan 6, 2023

Choose a reason for hiding this comment

djkoloski Jan 6, 2023

Choose a reason for hiding this comment

Add `set_max_level_racy` and gate `set_max_level` #544

Add `set_max_level_racy` and gate `set_max_level` #544

djkoloski commented Jan 6, 2023 •

edited

Loading