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

Write to #[thread_local] is not respected #54901

Closed
dtolnay opened this issue Oct 8, 2018 · 3 comments · Fixed by #57107
Closed

Write to #[thread_local] is not respected #54901

dtolnay opened this issue Oct 8, 2018 · 3 comments · Fixed by #57107
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled.

Comments

@dtolnay
Copy link
Member

dtolnay commented Oct 8, 2018

#![feature(thread_local)]

#[thread_local]
static S: &str = "before";

fn set_s() {
    S = "after";
}

fn main() {
    println!("{}", S);
    set_s();
    println!("{}", S);
}

On rustc 1.31.0-nightly (b2d6ea9 2018-10-07) and x86_64-unknown-linux-gnu, we see the following surprising behavior:

$ cargo run  # as expected
before
after

$ cargo run --release
before
before

In release mode it seems the write has not happened in the place that it should.

Mentioning thread_local tracking issue: rust-lang/rust#29594.

@dtolnay
Copy link
Member Author

dtolnay commented Oct 8, 2018

Ah, apparently this is missing a mut. With mut the behavior is correct. We need to make sure mutating a thread_local static that does not have mut does not compile.

#![feature(thread_local)]

#[thread_local]
static mut S: &str = "before";

fn set_s() {
    unsafe { S = "after" };
}

fn main() {
    println!("{}", unsafe { S });
    set_s();
    println!("{}", unsafe { S });
}
$ cargo run --release
before
after

@matthewjasper matthewjasper added the NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. label Oct 12, 2018
@eddyb
Copy link
Member

eddyb commented Nov 4, 2018

cc @pnkfelix @nikomatsakis Did #55150 fix this by any chance?

@pnkfelix
Copy link
Member

pnkfelix commented Nov 5, 2018

It looks like this is fixed on nightly; I'm guessing #55150 is the reason. We probably should add some regression tests for the cases listed here.

@pnkfelix pnkfelix added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Nov 5, 2018
Centril added a commit to Centril/rust that referenced this issue Jan 15, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
Centril added a commit to Centril/rust that referenced this issue Jan 17, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
Centril added a commit to Centril/rust that referenced this issue Jan 17, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
Centril added a commit to Centril/rust that referenced this issue Jan 17, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
Centril added a commit to Centril/rust that referenced this issue Jan 18, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
Centril added a commit to Centril/rust that referenced this issue Jan 18, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 18, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
Centril added a commit to Centril/rust that referenced this issue Jan 18, 2019
…tsakis

Add a regression test for mutating a non-mut #[thread_local]

This should close rust-lang#54901 since the regression has since been fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants