sync: Bring the library up to date #12900

alexcrichton · 2014-03-15T01:34:17Z

Remove clone-ability from all primitives. All shared state will now come
from the usage of the primitives being shared, not the primitives being
inherently shareable. This allows for fewer allocations for stack-allocated
primitives.
Add Mutex<T> and RWLock<T> which are stack-allocated primitives for purely
wrapping a piece of data
Remove RWArc<T> in favor of Arc<RWLock<T>>
Remove MutexArc<T> in favor of Arc<Mutex<T>>
Shuffle around where things are located
- The arc module now only contains Arc
- A new lock module contains Mutex, RWLock, and Barrier
- A new raw module contains the primitive implementations of Semaphore,
  Mutex, and RWLock
The Deref/DerefMut trait was implemented where appropriate
CowArc was removed, the functionality is now part of Arc and is tagged
with #[experimental].
The crate now has #[deny(missing_doc)]
Arc now supports weak pointers

This is not a large-scale rewrite of the functionality contained within the
sync crate, but rather a shuffling of who does what an a thinner hierarchy of
ownership to allow for better composability.

huonw · 2014-03-15T02:40:23Z

Travis has legitimate tidy failures.

(BTW, github was eating the type parameters in your description as HTML; I surrounded them with `'s, which seems to have fixed it.)

sfackler · 2014-03-15T04:08:31Z

src/libsync/arc.rs

    #[inline]
+    #[experimental]
    pub fn get_mut<'a>(&'a mut self) -> &'a mut T {


It'd be nice if this method name emphasized that it's a COW operation.

alexcrichton · 2014-03-15T04:24:35Z

A semantic difference that fell out as part of this rewrite is that you can nest rwlocks now. We're moving in the direction of not trying to prevent deadlock, so I think that this is fine.

huonw · 2014-03-15T04:30:35Z

src/libextra/workcache.rs

@@ -484,7 +484,7 @@ fn test() {

    let db_path = make_path(~"db.json");

-    let cx = Context::new(RWArc::new(Database::new(db_path)),
+    let cx = Context::new(Arc::new(RWLock::new(Database::new(db_path))),


I'm surprised that this is valid: shouldn't RWLock be not-Freeze? (I guess this is mostly irrelevant with Freeze disappearing, though.)

I believe that the Freeze bound was mostly for non-recursive locks, but we've basically stopped doing that now, and this explicitly wanted them to be Freeze so they could go in arcs.

The name Share is much more appropriate, and the usage mostly reflects what things will look like for the Freeze => Share transition.

My point is RWLock shouldn't be Freeze, aiui, since it has inner mutability (right?)... but it is Share so this isn't worth worrying about.

bill-myers · 2014-03-15T15:01:16Z

Awesome, finally the API gets in a mostly sensible shape.

Only thing that seems really objectionable is putting "condvar" inside MutexGuard, since most mutexes won't need condvars, and tying them is really inflexible (e.g. can't put them in a HashMap<~str, Condvar> but only a static array inside the mutex itself).

I think condvars need to be split from mutexes, take a guard parameter, and check at runtime that the guard is from the correct lock.

Also, naming seems debatable: maybe Mutex and RWLock should be renamed to something like SyncCell and RwSyncCell or perhaps just Sync and RwSync?

The current naming scheme in this pull request is akin to calling RefCell BorrowFlags instead.

flaper87 · 2014-03-15T16:05:14Z

src/libsync/lock.rs

+/// signaling, for use with the lock types.
+pub struct Condvar<'a> {
+    priv name: &'static str,
+    // n.b. Inner must be after PoisonOnFail becase we must set the poison flag


alexcrichton · 2014-03-15T20:12:41Z

I think condvars need to be split from mutexes, take a guard parameter, and check at runtime that the guard is from the correct lock.

This PR is aimed at a refactoring to get things into shape, I agree that condvars need to be revisited, but I'm going to do that as a separate PR.

alexcrichton · 2014-03-19T06:09:50Z

Added weak pointers for Arc, removed get functions, and renamed get_mut to make_unique

huonw · 2014-03-22T01:20:11Z

src/libsync/arc.rs

+/// # Example
+///
+/// In this example, a large vector of floats is shared between several tasks.
+/// With simple pipes, without Arc, a copy would have to be made for each task.


huonw · 2014-03-22T02:07:47Z

src/libsync/lock.rs

+        // Tests that when a downgrader hands off the "reader cloud" lock
+        // because of a contending reader, a writer can't race to get it
+        // instead, which would result in readers_and_writers. This tests
+        // the sync module rather than this one, but it's here because an


This comment seems to be outdated? (eg. there's no sync module any more)

huonw · 2014-03-22T02:41:55Z

Ok, read over pretty much the whole thing; although it would be (very) good for someone else to check the use of the various atomics and consistencies etc.

There are several undefined behaviour &self -> &mut self transmutes blocked on #11583, I think it would be very good for this to wait until that's fixed (i.e. #13036 lands) to avoid introducing UB.

huonw · 2014-03-22T02:49:04Z

(Also, it would be really neat if sync could get a #[deny(deprecated_owned_vector)] with these changes...)

alexcrichton · 2014-03-22T03:11:19Z

Thanks for the thorough review @huonw! I am quite eager to remove the transmutes blocked on #11583!

huonw · 2014-03-22T03:37:58Z

(Travis has relevant failures.)

alexcrichton · 2014-03-22T08:03:02Z

Updated with #13069 plus lots of migration from &mut self to &self

alexcrichton · 2014-03-22T08:03:24Z

A snapshot is being built which I hope to include in this PR to cut down on the cfg(stage0)

alexcrichton · 2014-03-22T15:51:34Z

Updated with new snapshots so there is no longer any cfg(stage0)

The proper usage of shared types is now sharing through `&self` rather than `&mut self` because the mutable version will provide stronger guarantees (no aliasing on *any* thread).

This commit also lifts it up a level in the module hierarchy in the soon-to-come reorganization of libsync.

This also uses the Unsafe type for any interior mutability in the type to avoid transmutes.

Similarly to the rest of the previous commits, this moves the once primitive to using &self instead of &mut self for proper sharing among many threads now.

This commit rewrites the core primitives of the sync library: Mutex, RWLock, and Semaphore. These primitives now have updated, more modernized apis: * Guards are returned instead of locking with closures. All condition variables have moved inside the guards and extraneous methods have been removed. * Downgrading on an rwlock is now done through the guard instead of the rwlock itself. These types are meant to be general locks, not locks of an internal type (for external usage). New types will be introduced for locking shared data.

This introduces new synchronization types which are meant to be the foundational building blocks for sharing data among tasks. The new Mutex and RWLock types have a type parameter which is the internal data that is accessed. Access to the data is all performed through the guards returned, and the guards all have autoderef implemented for easy access.

This removes the now-outdated MutexArc and RWArc types. These are superseded by Arc<Mutex<T>> and Arc<RWLock<T>>. The only remaining arc is the one true Arc. Additionally, the arc now has weak pointers implemented for it to assist in breaking cycles. This commit brings the arc api up to parity with the sibling Rc api, making them nearly interchangeable for inter and intra task communication.

This updates the exports and layout of the crate

* Remove clone-ability from all primitives. All shared state will now come from the usage of the primitives being shared, not the primitives being inherently shareable. This allows for fewer allocations for stack-allocated primitives. * Add `Mutex<T>` and `RWLock<T>` which are stack-allocated primitives for purely wrapping a piece of data * Remove `RWArc<T>` in favor of `Arc<RWLock<T>>` * Remove `MutexArc<T>` in favor of `Arc<Mutex<T>>` * Shuffle around where things are located * The `arc` module now only contains `Arc` * A new `lock` module contains `Mutex`, `RWLock`, and `Barrier` * A new `raw` module contains the primitive implementations of `Semaphore`, `Mutex`, and `RWLock` * The Deref/DerefMut trait was implemented where appropriate * `CowArc` was removed, the functionality is now part of `Arc` and is tagged with `#[experimental]`. * The crate now has #[deny(missing_doc)] * `Arc` now supports weak pointers This is not a large-scale rewrite of the functionality contained within the `sync` crate, but rather a shuffling of who does what an a thinner hierarchy of ownership to allow for better composability.

publish: Use cargo ws rename to rename crates Follow up for rust-lang#12716

sfackler reviewed Mar 15, 2014
View reviewed changes

huonw reviewed Mar 15, 2014
View reviewed changes

flaper87 reviewed Mar 15, 2014
View reviewed changes

alexcrichton mentioned this pull request Mar 16, 2014

std: Improve docs for atomics. Fix two bugs #12954

Merged

huonw reviewed Mar 22, 2014
View reviewed changes

huonw mentioned this pull request Mar 22, 2014

std: Unignore atomic tests #13069

Closed

alexcrichton added 7 commits March 23, 2014 09:45

Snapshot cleanup

da118e8

std: Move NativeMutex from &mut self to &self

dd64bd8

The proper usage of shared types is now sharing through `&self` rather than `&mut self` because the mutable version will provide stronger guarantees (no aliasing on *any* thread).

sync: Move the concurrent queue to using &self

d6b3f1f

This commit also lifts it up a level in the module hierarchy in the soon-to-come reorganization of libsync.

sync: Move the Mutex type to using &self

3572a30

This also uses the Unsafe type for any interior mutability in the type to avoid transmutes.

sync: Move Once to using &self

53e451f

Similarly to the rest of the previous commits, this moves the once primitive to using &self instead of &mut self for proper sharing among many threads now.

alexcrichton and others added 4 commits March 24, 2014 17:17

sync: Wire up all of the previous commits

eff0257

This updates the exports and layout of the crate

test: Update all tests with the sync changes

5163a26

std: Unignore atomic tests

218461d

bors closed this Mar 25, 2014

alexcrichton deleted the rewrite-sync branch March 25, 2014 17:07

matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this pull request Aug 2, 2022

Auto merge of rust-lang#12900 - pksunkara:master, r=lnicola

af2b806

publish: Use cargo ws rename to rename crates Follow up for rust-lang#12716

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sync: Bring the library up to date #12900

sync: Bring the library up to date #12900

alexcrichton commented Mar 15, 2014

huonw commented Mar 15, 2014

sfackler Mar 15, 2014

brson Mar 16, 2014

alexcrichton commented Mar 15, 2014

huonw Mar 15, 2014

alexcrichton Mar 15, 2014

huonw Mar 15, 2014

bill-myers commented Mar 15, 2014

flaper87 Mar 15, 2014

alexcrichton commented Mar 15, 2014

alexcrichton commented Mar 19, 2014

huonw Mar 22, 2014

huonw Mar 22, 2014

huonw commented Mar 22, 2014

huonw commented Mar 22, 2014

alexcrichton commented Mar 22, 2014

huonw commented Mar 22, 2014

alexcrichton commented Mar 22, 2014

alexcrichton commented Mar 22, 2014

alexcrichton commented Mar 22, 2014

sync: Bring the library up to date #12900

sync: Bring the library up to date #12900

Conversation

alexcrichton commented Mar 15, 2014

huonw commented Mar 15, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton commented Mar 15, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bill-myers commented Mar 15, 2014

Choose a reason for hiding this comment

alexcrichton commented Mar 15, 2014

alexcrichton commented Mar 19, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huonw commented Mar 22, 2014

huonw commented Mar 22, 2014

alexcrichton commented Mar 22, 2014

huonw commented Mar 22, 2014

alexcrichton commented Mar 22, 2014

alexcrichton commented Mar 22, 2014

alexcrichton commented Mar 22, 2014