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

Remove the UnsafeArc type #14301

Merged
merged 13 commits into from
May 22, 2014
Merged

Conversation

alexcrichton
Copy link
Member

This type can be built with Arc<Unsafe<T>> now that liballoc exists.

This also removes the `&mut self` requirement, using the correct `&self`
requirement for concurrent types.
This removes usage of UnsafeArc and uses proper self mutability for concurrent
types.
This removes the incorrect `&mut self` taken because it can alias among many
threads.
This removes the incorrect usage of `&mut self` in a concurrent setting.
This removes the usage of UnsafeArc
They currently still use `&mut self`, this migration was aimed towards moving
from UnsafeArc<T> to Arc<Unsafe<T>>
This type has been superseded by Arc<Unsafe<T>>. The UnsafeArc type is a relic
of an era that has long since past, and with the introduction of liballoc the
standard library is able to use the Arc smart pointer. With little need left for
UnsafeArc, it was removed.

All existing code using UnsafeArc should either be reevaluated to whether it can
use only Arc, or it should transition to Arc<Unsafe<T>>

[breaking-change]
@lilyball
Copy link
Contributor

I guess that comment applies generally, not just to the first commit. All of the types that you're enabling mutation through &self on, are you satisfied that they're thread-safe, or should they be marked NoShare?

@alexcrichton
Copy link
Member Author

These types are no more or less safe than they were before, because they were impossible to safely be used in a shared context (all methods took &mut). They are not 100% safe, and I don't want them as part of the public api of libstd or libsync, and they will likely become #[experimental] or some similar method.

I'm currently debugging a crash which is caused by this.

@lilyball
Copy link
Contributor

@alexcrichton Right, but they took &mut, which presumably meant they couldn't be mutated simultaneously on multiple threads. They now take &. This is why I'm asking if they should be marked as NoShare.

@alexcrichton
Copy link
Member Author

It's not very helpful for concurrent queues to be NoShare? These types are all experimental, they're not 100% safe either before or after this patch.

@brson
Copy link
Contributor

brson commented May 20, 2014

I've been anticipating this day for so long!

@lilyball
Copy link
Contributor

@alexcrichton I was thinking more along the lines of Worker/Stealer being NoShare. Are those meant to be concurrent? I was assuming they were task-local values that shared a concurrent queue.

@alexcrichton
Copy link
Member Author

Managed to fix the crash I was seeing, and I marked the types that did the work of being split into halves to have each half marked as NoShare (if only we had opt-in builtin types...)

Build is still running, but should be good for a go now.

bors added a commit that referenced this pull request May 22, 2014
This type can be built with `Arc<Unsafe<T>>` now that liballoc exists.
@bors bors closed this May 22, 2014
@bors bors merged commit fdf935a into rust-lang:master May 22, 2014
@alexcrichton alexcrichton deleted the remove-unsafe-arc branch May 22, 2014 04:14
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 this pull request may close these issues.

4 participants