Skip to content

Commit

Permalink
Auto merge of #45198 - oli-obk:fmt_args, r=sfackler
Browse files Browse the repository at this point in the history
Prevent fmt::Arguments from being shared across threads

Fixes #45197

This is a **breaking change**! Without doing this it's very easy to create race conditions.

There's probably a way to do this without breaking valid use cases, but it would require quite an overhaul of the formatting machinery.
  • Loading branch information
bors committed Nov 22, 2017
2 parents 3755fe9 + dc7de37 commit 1dc0b57
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/libcore/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,14 @@ pub struct Formatter<'a> {

struct Void {
_priv: (),
/// Erases all oibits, because `Void` erases the type of the object that
/// will be used to produce formatted output. Since we do not know what
/// oibits the real types have (and they can have any or none), we need to
/// take the most conservative approach and forbid all oibits.
///
/// It was added after #45197 showed that one could share a `!Sync`
/// object across threads by passing it into `format_args!`.
_oibit_remover: PhantomData<*mut Fn()>,
}

/// This struct represents the generic "argument" which is taken by the Xprintf
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/fmt/send-sync.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn send<T: Send>(_: T) {}
fn sync<T: Sync>(_: T) {}

fn main() {
// `Cell` is not `Sync`, so `&Cell` is neither `Sync` nor `Send`,
// `std::fmt::Arguments` used to forget this...
let c = std::cell::Cell::new(42);
send(format_args!("{:?}", c));
sync(format_args!("{:?}", c));
}
34 changes: 34 additions & 0 deletions src/test/ui/fmt/send-sync.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error[E0277]: the trait bound `*mut std::ops::Fn() + 'static: std::marker::Sync` is not satisfied in `[std::fmt::ArgumentV1<'_>]`
--> $DIR/send-sync.rs:18:5
|
18 | send(format_args!("{:?}", c));
| ^^^^ `*mut std::ops::Fn() + 'static` cannot be shared between threads safely
|
= help: within `[std::fmt::ArgumentV1<'_>]`, the trait `std::marker::Sync` is not implemented for `*mut std::ops::Fn() + 'static`
= note: required because it appears within the type `std::marker::PhantomData<*mut std::ops::Fn() + 'static>`
= note: required because it appears within the type `core::fmt::Void`
= note: required because it appears within the type `&core::fmt::Void`
= note: required because it appears within the type `std::fmt::ArgumentV1<'_>`
= note: required because it appears within the type `[std::fmt::ArgumentV1<'_>]`
= note: required because of the requirements on the impl of `std::marker::Send` for `&[std::fmt::ArgumentV1<'_>]`
= note: required because it appears within the type `std::fmt::Arguments<'_>`
= note: required by `send`

error[E0277]: the trait bound `*mut std::ops::Fn() + 'static: std::marker::Sync` is not satisfied in `std::fmt::Arguments<'_>`
--> $DIR/send-sync.rs:19:5
|
19 | sync(format_args!("{:?}", c));
| ^^^^ `*mut std::ops::Fn() + 'static` cannot be shared between threads safely
|
= help: within `std::fmt::Arguments<'_>`, the trait `std::marker::Sync` is not implemented for `*mut std::ops::Fn() + 'static`
= note: required because it appears within the type `std::marker::PhantomData<*mut std::ops::Fn() + 'static>`
= note: required because it appears within the type `core::fmt::Void`
= note: required because it appears within the type `&core::fmt::Void`
= note: required because it appears within the type `std::fmt::ArgumentV1<'_>`
= note: required because it appears within the type `[std::fmt::ArgumentV1<'_>]`
= note: required because it appears within the type `&[std::fmt::ArgumentV1<'_>]`
= note: required because it appears within the type `std::fmt::Arguments<'_>`
= note: required by `sync`

error: aborting due to 2 previous errors

0 comments on commit 1dc0b57

Please sign in to comment.