
Reference cycle bug in Rust's scoped threads - doomrobo
https://github.com/rust-lang/rust/issues/24292
======
aturon
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).

~~~
anon4
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?

~~~
steveklabnik
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.

------
ekimekim
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.

~~~
sctb
We updated the title to hopefully be less misleading.

~~~
ekimekim
Much better, thank you.

------
carllerche
`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.

~~~
dbaupp
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.

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

~~~
kibwen
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.

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

~~~
madez
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.

------
doomrobo
Discussion on reddit:
[http://www.reddit.com/r/rust/comments/32jmf8/stdthreadscoped...](http://www.reddit.com/r/rust/comments/32jmf8/stdthreadscoped_found_to_be_unsound/)

------
adrusi
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.

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

~~~
zmanian
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...](http://blog.rust-lang.org/2015/04/10/Fearless-
Concurrency.html)

~~~
CarlColglazier
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.

