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

std::io::Error is Send but not Sync #24049

Closed
lilyball opened this issue Apr 4, 2015 · 5 comments · Fixed by #24133
Closed

std::io::Error is Send but not Sync #24049

lilyball opened this issue Apr 4, 2015 · 5 comments · Fixed by #24133

Comments

@lilyball
Copy link
Contributor

lilyball commented Apr 4, 2015

The std::io::Error type is Send but it's not Sync. This is because of the use of Box<Error+Send>, as that value is not Sync, and so therefore io::Error is not Sync.

This is a problem for me as I want to stuff an io::Error into a sync::Arc and have the resulting value be Send, but Arc is only Send if its element type is Send + Sync.

/cc @alexcrichton

@alexcrichton
Copy link
Member

This is currently an intentional decision as by using a trait object it will always be opting out of some marker traits. The only real fundamental piece of functionality we wanted to add to io::Error was the ability to send it across threads, hence Send. That said we can always tweak this, however.

@alexcrichton
Copy link
Member

I'm starting to think that putting an error in an Arc is a pretty important use case, in which case I would be fine going ahead and adding the Sync bound to io::Error

@lilyball
Copy link
Contributor Author

lilyball commented Apr 6, 2015

I'm still in favor of just making io::Error use Arc directly instead of Box (which still requires the Sync bound), because then I could just go back to calling .clone() on io::Error instead of using a custom Error type that pretends to be the underlying shared io::Error.

@lilyball
Copy link
Contributor Author

lilyball commented Apr 7, 2015

It occurs to me that, AFAIK, we can't use smart pointers with trait objects yet. So we can't actually say Arc<error::Error+Send+Sync>. io::Error currently has a Box-in-a-Box so we could use Arc on the outer Box, although I don't see any good reason for it to have two Boxes right now at all (AFAICT all it's doing is saving one word from the size of io::Errors initialized with OS error codes).

@lilyball
Copy link
Contributor Author

lilyball commented Apr 7, 2015

#24133 makes io::Error implement Sync. I've filed #24135 for the proposal to make io::Error use Arc internally so it can conform to Clone.

lilyball added a commit that referenced this issue Apr 18, 2015
This allows `io::Error` values to be stored in `Arc` properly.

Because this requires `Sync` of any value passed to `io::Error::new()`
and modifies the relevant `convert::From` impls, this is a

[breaking-change]

Fixes #24049.
bors added a commit that referenced this issue Apr 18, 2015
This allows `io::Error` values to be stored in `Arc` properly.

Because this requires `Sync` of any value passed to `io::Error::new()`
and modifies the relevant `convert::From` impls, this is a

[breaking-change]

Fixes #24049.
alexcrichton pushed a commit to alexcrichton/rust that referenced this issue Apr 23, 2015
This allows `io::Error` values to be stored in `Arc` properly.

Because this requires `Sync` of any value passed to `io::Error::new()`
and modifies the relevant `convert::From` impls, this is a

[breaking-change]

Fixes rust-lang#24049.
alexcrichton pushed a commit to alexcrichton/rust that referenced this issue Apr 23, 2015
This allows `io::Error` values to be stored in `Arc` properly.

Because this requires `Sync` of any value passed to `io::Error::new()`
and modifies the relevant `convert::From` impls, this is a

[breaking-change]

Fixes rust-lang#24049.
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

Successfully merging a pull request may close this issue.

3 participants