-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
use Once instead of Mutex to manage capture resolution #80736
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
|
||
// SAFETY: Access to the inner value is synchronized using a thread-safe `Once` | ||
// So long as `Capture` is `Sync`, `LazilyResolvedCapture` is too | ||
unsafe impl Sync for LazilyResolvedCapture where Capture: Sync {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not visible in the diff, but we do have a test to ensure Backtrace
is Send + Sync
This comment has been minimized.
This comment has been minimized.
This allows us to return borrows of the captured backtrace frames that are tied to a borrow of the Backtrace itself, instead of to some short-lived Mutex guard. It also makes it semantically clearer what synchronization is needed on the capture.
LGTM |
r? @dtolnay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@bors r+ |
📌 Commit db4585a has been approved by |
Rollup of 10 pull requests Successful merges: - rust-lang#78901 (diagnostics: Note capturing closures can't be coerced to fns) - rust-lang#79588 (Provide more information for HRTB lifetime errors involving closures) - rust-lang#80232 (Remove redundant def_id lookups) - rust-lang#80662 (Added support for i386-unknown-linux-gnu and i486-unknown-linux-gnu) - rust-lang#80736 (use Once instead of Mutex to manage capture resolution) - rust-lang#80796 (Update to LLVM 11.0.1) - rust-lang#80859 (Fix --pretty=expanded with --remap-path-prefix) - rust-lang#80922 (Revert "Auto merge of rust-lang#76896 - spastorino:codegen-inline-fns2) - rust-lang#80924 (Fix rustdoc --test-builder argument parsing) - rust-lang#80935 (Rename `rustc_middle::lint::LevelSource` to `LevelAndSource`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
For #78299
This allows us to return borrows of the captured backtrace frames that are tied to a borrow of the Backtrace itself, instead of to some short-lived Mutex guard.
We could alternatively share
&Mutex<Capture>
s and lock on-demand, but then we could potentially forget to callresolve()
before working with the capture. It also makes it semantically clearer what synchronization is needed on the capture.cc @seanchen1991 @rust-lang/project-error-handling