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

Can put non-Sync data in extern statics #40652

Closed
japaric opened this issue Mar 19, 2017 · 4 comments
Closed

Can put non-Sync data in extern statics #40652

japaric opened this issue Mar 19, 2017 · 4 comments

Comments

@japaric
Copy link
Member

japaric commented Mar 19, 2017

This is not allowed:

static FOO: Cell<i32> = Cell::new(0);
error[E0277]: the trait bound `std::cell::Cell<i32>: std::marker::Sync` is not satisfied
 --> src/lib.rs:5:1
  |
5 | static FOO: Cell<i32> = Cell::new(0);
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Sync` is not implemented for `std::cell::Cell<i32>`
  |
  = note: `std::cell::Cell<i32>` cannot be shared between threads safely
  = note: shared static variables must have a type that implements `Sync`

But this is allowed:

use std::cell::Cell;

extern "C" {
    static FOO: Cell<i32>;
}
warning: found struct without foreign-function-safe representation annotation in foreign module, consider adding a #[repr(C)] attribute to the type
 --> src/lib.rs:4:17
  |
4 |     static FOO: Cell<i32>;
  |                 ^^^^^^^^^
  |
  = note: #[warn(improper_ctypes)] on by default

Using extern "C" doesn't suddenly make FOO safe to share accross threads so this should become an error, IMO. Just like #35112.

cc @nikomatsakis

@retep998
Copy link
Member

As a result of #35112, extern statics currently require unsafe to access so this isn't a safety hole. Also, extern statics are often used to reference raw pointers which are definitely !Sync, so changing this to be an error would be a rather huge breaking change.

In my opinion this is working as intended.

@japaric
Copy link
Member Author

japaric commented Mar 20, 2017

As a result of #35112, extern statics currently require unsafe to access so this isn't a safety hole

I agree.

Also, extern statics are often used to reference raw pointers which are definitely !Sync

There's a pattern for such thing:

pub struct NotSyncAtAll<T>(*mut T);

unsafe impl<T> Sync for NotSyncAtAll<T> {}

extern "C" {
    static FOO: NotSyncAtAll<u8>;
}

But if FOO is still unsafe to access after that then maybe that doesn't buy you much. And I don't think extern statics can't ever be safe in the general case as there's no type safety; you can't bind any type to FOO.

so changing this to be an error would be a rather huge breaking change.

Well, that's one reason to leave things as they are.

@nikomatsakis
Copy link
Contributor

I'm also inclined to think this is working as intended.

@japaric
Copy link
Member Author

japaric commented Mar 28, 2017

Agree that the change doesn't seem like it would buy anything and it would just result in breakage so closing.

@japaric japaric closed this as completed Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants