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

Tracking Issue for Mutex and RWLock unique accessors #28968

Closed
4 tasks
Gankra opened this issue Oct 11, 2015 · 11 comments · Fixed by #30187
Closed
4 tasks

Tracking Issue for Mutex and RWLock unique accessors #28968

Gankra opened this issue Oct 11, 2015 · 11 comments · Fixed by #30187
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Gankra
Copy link
Contributor

Gankra commented Oct 11, 2015

After #29031, this is now the tracking issue for the stabilization of the following features:

  • mutex_into_inner
  • mutex_get_mut
  • rwlock_into_inner
  • rwlock_get_mut

Original report

As far as I can tell, it's sound to straight-up-unwrap a Mutex if you have it by value. This would enable one to do:

let data = Mutex::new(vec![])
crossbeam::scope(|scope| {
  for input in inputs {
    let data = &data;
    scope.spawn(move || {
      let output = expensive_computation(input);
      data.lock().unwrap().push(output);
    }
  }
});

for expensive in data.into_inner() {
  non_thread_safe(expensive);
}

Which today requires mutexing an &mut:

let mut data = vec![];
{
  let data_mutex = Mutex::new(&mut data);
  crossbeam::scope(|scope| {
    for input in inputs {
      let data = &data_mutex;
      scope.spawn(move || {
        let output = expensive_computation(input);
        data.lock().unwrap().push(output);
      }
    }
  });
}

for expensive in data.into_inner() {
  non_thread_safe(expensive);
}

Given Arc can be unwrapped, this would also allow the last Arc<Mutex<T>> in some concurrent context to be fully unwrapped to a T.

@Gankra
Copy link
Contributor Author

Gankra commented Oct 11, 2015

cc @aturon

@sfackler
Copy link
Member

Seems like just an oversight to me.

@alexcrichton
Copy link
Member

This is a dupe of rust-lang/rfcs#1269 (although cross-repo dupes are always interesting).

The consensus is basically "this can be added as soon as someone puts in the effort".

@Gankra Gankra added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Oct 11, 2015
@Gankra
Copy link
Contributor Author

Gankra commented Oct 11, 2015

Seems like a good noobie task (upon reflection I agree with the rfc issue's assesment that this should return a poison guard).

@cristicbz
Copy link
Contributor

On it!

bors added a commit that referenced this issue Oct 15, 2015
The implementation for `into_inner` was a bit more complex than I had hoped for---is there any simpler, less unsafe way of getting around the fact that one can't move out of a `Drop` struct?

See #28968 and rust-lang/rfcs#1269 .
@cristicbz
Copy link
Contributor

Edit: removed since @gankro changed original description

After #29031, this is now the tracking issue for the stabilization of the following features:

  • mutex_into_inner
  • mutex_get_mut
  • rwlock_into_inner
  • ~~rwlock_get_mut~~~

@cristicbz
Copy link
Contributor

This should probably be renamed into Tracking issue for sync into_inner and get_mut or something.

@Gankra Gankra changed the title Mutex::into_inner Tracking Issue for Mutex and RWLock unique accessors Oct 16, 2015
@alexcrichton alexcrichton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Oct 16, 2015
@alexcrichton
Copy link
Member

Nominating for 1.6 discussion

@alexcrichton
Copy link
Member

🔔 This issue is now entering its cycle-long final comment period for stabilization 🔔

@alexcrichton alexcrichton added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed I-nominated labels Nov 5, 2015
@aturon
Copy link
Member

aturon commented Dec 1, 2015

I'm in favor. Follows existing conventions, fills in a bit of missing expressiveness.

@cristicbz
Copy link
Contributor

Something to consider that I didn't when I submitted the PR is whether we also want fn get_mut(&mut self) on RefCell (into_inner already exists there). In addition to consistency, I actually stumbled upon this in real code: I'm using a RefCell to reuse an allocation (cached as a member) in &self methods, but I also need use it in some of the &mut self methods.

Drawbacks: the borrow_mut / get_mut duality is bound to be confusing.

(crazy idea: it would be really awesome actually, if RefCell, Mutex etc. could somehow magically implement DerefMut without Deref. Something like a trait UniqueDeref { type Target; fn unique_deref(&mut self) -> &mut Self::Target } that would somehow prevent Deref impl-s).

alexcrichton added a commit to alexcrichton/rust that referenced this issue Dec 5, 2015
This commit is the standard API stabilization commit for the 1.6 release cycle.
The list of issues and APIs below have all been through their cycle-long FCP and
the libs team decisions are listed below

Stabilized APIs

* `Read::read_exact`
* `ErrorKind::UnexpectedEof` (renamed from `UnexpectedEOF`)
* libcore -- this was a bit of a nuanced stabilization, the crate itself is now
  marked as `#[stable]` and the methods appearing via traits for primitives like
  `char` and `str` are now also marked as stable. Note that the extension traits
  themeselves are marked as unstable as they're imported via the prelude. The
  `try!` macro was also moved from the standard library into libcore to have the
  same interface. Otherwise the functions all have copied stability from the
  standard library now.
* The `#![no_std]` attribute
* `fs::DirBuilder`
* `fs::DirBuilder::new`
* `fs::DirBuilder::recursive`
* `fs::DirBuilder::create`
* `os::unix::fs::DirBuilderExt`
* `os::unix::fs::DirBuilderExt::mode`
* `vec::Drain`
* `vec::Vec::drain`
* `string::Drain`
* `string::String::drain`
* `vec_deque::Drain`
* `vec_deque::VecDeque::drain`
* `collections::hash_map::Drain`
* `collections::hash_map::HashMap::drain`
* `collections::hash_set::Drain`
* `collections::hash_set::HashSet::drain`
* `collections::binary_heap::Drain`
* `collections::binary_heap::BinaryHeap::drain`
* `Vec::extend_from_slice` (renamed from `push_all`)
* `Mutex::get_mut`
* `Mutex::into_inner`
* `RwLock::get_mut`
* `RwLock::into_inner`
* `Iterator::min_by_key` (renamed from `min_by`)
* `Iterator::max_by_key` (renamed from `max_by`)

Deprecated APIs

* `ErrorKind::UnexpectedEOF` (renamed to `UnexpectedEof`)
* `OsString::from_bytes`
* `OsStr::to_cstring`
* `OsStr::to_bytes`
* `fs::walk_dir` and `fs::WalkDir`
* `path::Components::peek`
* `slice::bytes::MutableByteVector`
* `slice::bytes::copy_memory`
* `Vec::push_all` (renamed to `extend_from_slice`)
* `Duration::span`
* `IpAddr`
* `SocketAddr::ip`
* `Read::tee`
* `io::Tee`
* `Write::broadcast`
* `io::Broadcast`
* `Iterator::min_by` (renamed to `min_by_key`)
* `Iterator::max_by` (renamed to `max_by_key`)
* `net::lookup_addr`

New APIs (still unstable)

* `<[T]>::sort_by_key` (added to mirror `min_by_key`)

Closes rust-lang#27585
Closes rust-lang#27704
Closes rust-lang#27707
Closes rust-lang#27710
Closes rust-lang#27711
Closes rust-lang#27727
Closes rust-lang#27740
Closes rust-lang#27744
Closes rust-lang#27799
Closes rust-lang#27801
cc rust-lang#27801 (doesn't close as `Chars` is still unstable)
Closes rust-lang#28968
bors added a commit that referenced this issue Dec 6, 2015
This commit is the standard API stabilization commit for the 1.6 release cycle.
The list of issues and APIs below have all been through their cycle-long FCP and
the libs team decisions are listed below

Stabilized APIs

* `Read::read_exact`
* `ErrorKind::UnexpectedEof` (renamed from `UnexpectedEOF`)
* libcore -- this was a bit of a nuanced stabilization, the crate itself is now
  marked as `#[stable]` and the methods appearing via traits for primitives like
  `char` and `str` are now also marked as stable. Note that the extension traits
  themeselves are marked as unstable as they're imported via the prelude. The
  `try!` macro was also moved from the standard library into libcore to have the
  same interface. Otherwise the functions all have copied stability from the
  standard library now.
* `fs::DirBuilder`
* `fs::DirBuilder::new`
* `fs::DirBuilder::recursive`
* `fs::DirBuilder::create`
* `os::unix::fs::DirBuilderExt`
* `os::unix::fs::DirBuilderExt::mode`
* `vec::Drain`
* `vec::Vec::drain`
* `string::Drain`
* `string::String::drain`
* `vec_deque::Drain`
* `vec_deque::VecDeque::drain`
* `collections::hash_map::Drain`
* `collections::hash_map::HashMap::drain`
* `collections::hash_set::Drain`
* `collections::hash_set::HashSet::drain`
* `collections::binary_heap::Drain`
* `collections::binary_heap::BinaryHeap::drain`
* `Vec::extend_from_slice` (renamed from `push_all`)
* `Mutex::get_mut`
* `Mutex::into_inner`
* `RwLock::get_mut`
* `RwLock::into_inner`
* `Iterator::min_by_key` (renamed from `min_by`)
* `Iterator::max_by_key` (renamed from `max_by`)

Deprecated APIs

* `ErrorKind::UnexpectedEOF` (renamed to `UnexpectedEof`)
* `OsString::from_bytes`
* `OsStr::to_cstring`
* `OsStr::to_bytes`
* `fs::walk_dir` and `fs::WalkDir`
* `path::Components::peek`
* `slice::bytes::MutableByteVector`
* `slice::bytes::copy_memory`
* `Vec::push_all` (renamed to `extend_from_slice`)
* `Duration::span`
* `IpAddr`
* `SocketAddr::ip`
* `Read::tee`
* `io::Tee`
* `Write::broadcast`
* `io::Broadcast`
* `Iterator::min_by` (renamed to `min_by_key`)
* `Iterator::max_by` (renamed to `max_by_key`)
* `net::lookup_addr`

New APIs (still unstable)

* `<[T]>::sort_by_key` (added to mirror `min_by_key`)

Closes #27585
Closes #27704
Closes #27707
Closes #27710
Closes #27711
Closes #27727
Closes #27740
Closes #27744
Closes #27799
Closes #27801
cc #27801 (doesn't close as `Chars` is still unstable)
Closes #28968
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants