Reference cycle bug in Rust's scoped threads | Hacker News Reader

Reference cycle bug in Rust's scoped threads

github.com

106 pointsdoomrobo23 comments

To be clear, the basic concept of scoped threads is fine, and this unsoundness was a result of explicitly `unsafe` code.

The specific API chosen, however, used RAII guards to "guarantee" that the parent thread joined on the child thread within a certain scope. Unfortunately, it's possible to subvert the RAII guard by placing it within a ref-counted cycle, which results in its destructor never being run.

For the moment, the API is being moved back to "unstable" status.

We are considering a variety of ways to address the problem, including:

* Avoiding RAII guards in the API, instead using explicit closures to guarantee that the threads join.

* Introducing a way of marking types as "unsafe to leak", thereby allowing us to continue using the RAII-style API, but preventing use of the guard within reference counted pointers (which can leak).

I'm wondering -- would it be possible to have a cycle checker which would be run on function exit on the objects, for which the compiler can't prove, that they don't contain cyclic references?

In theory, things like this are possible. In practice, that's effectively a garbage collector, and current Rust is trying to take no-GC as far as possible.

Running cycle collection on function exit would make Rust tremendously slow. It's possible, but not a viable solution.

I dislike the post title as it makes this appear a bigger deal than it is. It should read "bug found in Rust scoped threads". The general principle has not been found unsound.

We updated the title to hopefully be less misleading.

Much better, thank you.

`thread::scoped` attempts to use lifetimes in order to enforce that values on the parent thread's stack would live throughout the life of the child thread, such that it would be safe to pass references to values on the parent thread to the child thread. However, the actual implementation used unsafe blocks. While this is unfortunate (thread::scoped was pretty cool), it isn't a hit against the Rust borrow checker.

It's not immediately obvious, but this isn't actually a problem with the idea of passing references to child threads. As background, `scoped` returns an RAII token and safety (references remaining valid while the child thread runs) is enforced via a destructor on that token. The issue is that reference counted types can have cycles, which are never destroyed, so placing the token in a reference counted cycle will stop the destructor running and hence risks unsafety.

It's fairly non-trivial to achieve this unsafety, but even the slightest risk is not acceptable for safe functions in the standard library, so we aren't going to ship it as is.

Fortunately, there are a few ways we can adjust things so that the interaction between reference counting and scoped threads doesn't cause problems, e.g. modify the reference counted types so that they cannot store types that require that the destructor be run like the `scoped` token, or guarantee safety in a way that doesn't hinge on the destructor of an object the user has ownership of.

Why is this such a big thing? There are still bugs being fixed to this day as Rust is still in beta.

It's a demonstration of the Rust developers putting their money where their mouth is by taking very seriously the notion of memory safety. The C++ approach would be to conclude that misuse of the API to a degree sufficient to cause memory unsafety is an error on behalf of the programmer, and that one should merely be careful. And it's absolutely true, the steps required to violate memory safety in this example are quite contrived. But Rust makes the claim that memory unsafety can't arise outside of an `unsafe` block, and their dedication to upholding that claim is laudable.

thanks for explanation. It should be description to this post, otherwise it looks like mirroring of github issues list to HN.

Why drag C++ into this? The rest of your answer holds up even without the crack at C++.

Because if C++ didn't have these problems, Rust wouldn't need to exist in the first place?

Maybe because if C++ would have been safer, there would be no Rust.

Rust is in direct competition to C++ and aims to be able to replace it completely. Therefore, Rust needs the direct comparison to C++ to show how it's better.

Discussion on reddit: http://www.reddit.com/r/rust/comments/32jmf8/stdthreadscoped...

As joshtriplett said in the issues, this depends on the already-unsafe ability of Rc to leak when a cycle is formed. It's just turning one form of unsafeness into a wide form. Nice to see that the team are doing what they can to about the issue though.

Don't understand why this bug deserves thread on HN.

Safe Fork-join concurrency which the entire stack frame is available to the new thread is a very sexy Rust feature.

It's unfortunate that this feature won't make into 1.0. It was one of the more exciting concurrency features that looked like it would be in 1.0 until this bug.

See Steve's last example here. http://blog.rust-lang.org/2015/04/10/Fearless-Concurrency.ht...

Since a new minor release is expected to come out every six weeks, it is possible that this will be added even after 1.0 at some point in the near future.

That post was written by Aauron Turon, not me. Though I have been talking about scoped threads a lot lately :)

Up votes by people who don't really buy all the Rust marketing about safety of the language, at least how it's gone about.

Personally speaking, and looking others threads on /r/rust, there is a fair amount of frustration when certain things don't work expected due to borrow checker (lifetimes, lexical scoping).

So when people who try Rust get a fair amount of smugness about safety when working with it... they will upvote stories where safety has been proved unsound in Rust.

Except Rust-the-language hasn't been proved unsound. Rust's standard library has been found to introduce unsoundness through the incorrect use of `unsafe` blocks. `thread::scoped`'s interface just has to be tweaked somehow, not removed.

Reply on news.ycombinator.com