Skip to content

Remove unwrap_shared_mutable_state #4436

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.

Sign up for GitHub

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

Jump to bottom
Closed
brson opened this issue Jan 11, 2013 · 7 comments
Closed

Remove unwrap_shared_mutable_state #4436

brson opened this issue Jan 11, 2013 · 7 comments
Labels
A-concurrency Area: Concurrency E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Milestone
0.7

Comments

@brson
Copy link
Contributor

brson commented Jan 11, 2013

We're about to start rewriting the runtime in Rust and need concurrency primitives, particulary atomic reference counting for schedulers, etc. This one feature makes some complicated calls into the runtime, making most of our primitives unusable for this task.
@bblum
Copy link
Contributor

bblum commented Jan 11, 2013

Removing unwrap will mean that once data is put into an ARC, MutexARC, or RWARC, it can never be owned again short of a deep copy.

I don't understand the issue here at all, though. It doesn't use any rustrt calls, only intrinsics (transmute, forget, cmpxchg). What's the problem?

@brson
Copy link
Contributor Author

brson commented Jan 11, 2013

That's fine with me. I don't have any use cases yet where I need to stop atomically reference counting something.

It interacts with the runtime because it uses pipes, which must be able to yield, resume, etc., task::killable, task::failing.

@bblum
Copy link
Contributor

bblum commented Jan 11, 2013

Oh, I see, you're saying there could be a circular functionality dependency. Why not just abstain from using it in the runtime's use cases, leaving it in for higher-level applications?

@brson
Copy link
Contributor Author

brson commented Feb 4, 2013

Mostly because that's not such a clean design. I'll also note that unwrap has bugs: #4689

@bblum
Copy link
Contributor

bblum commented Feb 4, 2013

I dunno. I think unwrap is a useful enough extension to the primitive (it enables a quick fork-many/join-many pattern) to warrant keeping.

As for the bug... ouch. I'll try to have a look at it soon. Hopefully just a bug in the test case.

@bblum
Copy link
Contributor

bblum commented Feb 28, 2013

Given that #4689 is fixed, I'm gonna vote for WONTFIXing this.

brson added a commit to brson/rust that referenced this issue Feb 28, 2013
@brson
core: Remove unwrap_shared_mutable_state. rust-lang#4436
78d5091
@brson brson mentioned this issue Feb 28, 2013
Closed
bors added a commit that referenced this issue Mar 1, 2013
@bors
auto merge of #5176 : brson/rust/unwrap_shared_mutable_state, r=nikom…
916d1a9 
…atsakis

r?

This fixes the current [random failures](http://buildbot.rust-lang.org/builders/auto-linux/builds/291/steps/test/logs/stdio) on the bots and closes #4436 by removing `unwrap_shared_mutable_state` and the code that depends on it. The result is that ARC-like things will not be unwrappable. This feature is complex and is not used outside of test cases.

Note that there is not consensus to remove it.

(second commit)
@brson
Copy link
Contributor Author

brson commented Mar 1, 2013

I've done this against your objection, @bblum. Sorry.

@brson brson closed this as completed Mar 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-concurrency Area: Concurrency E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Milestone
0.7
Development

No branches or pull requests
2 participants
@brson @bblum