Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Call Scott Meyers.

So you cannot safely assume your destructors will be called, and you need to proactively take steps like this in case they aren't. Nothing in the natural use of the language cues you to do this, so you need a moral guide like "Effective C++" for Rust.

This was the kind of accidental complexity that Rust had hoped to avoid. It seems like Rust has avoided a lot of it. But I guess it was entirely too optimistic to hope Rust could avoid all of it.



Note that this is only something you really need to think about when writing `unsafe` code, which basically unlocks a whole new set of rules that you need to worry about (zero-sized types in offsets, manual allocations, safety boundaries, GEP/aliasing rules, uninit memory, double destructors, etc). This is a drop in the bucket that anyone venturing to use `unsafe` will have to face in some kind of "guide to unsafety".


The problem is that all of this is safe code. The problem, to my mind, is that you can write mem::forget (as in the code here's safe_forget)* without ever using the unsafe keyword*.

Edit: I was wrong. The problem isn't with forget. This is hairy.


The 'mem::forget can be written in safe code' meme comes from https://github.com/rust-lang/rust/issues/24456

Which, as you can see, uses Rc, which uses unsafe code in a way that leads to the soundness bug. So that's not strictly true, or rather, doesn't really change anything, as it relies on the same bug.


nikomatsakis, in that thread:

"1. There were other ways to forget even before Rc, at least in some cases. For example, if T:Send holds, you could send the value to a thread that runs an infinite loop, or which is deadlocked on a port.

"2. It is true that one cannot assume that a destructor will run, and hence that forget is not itself unsafe (rather, it is unsafe to write a dtor that must run)...."

And alexcrichton:

"I commented on #24292, but the gist is that there are multiple ways to leak memory today (e.g. #14875 and #16135), so a targeted solution at Rc may not cover all use cases. Although as I mention in #24292 these other bugs can also be considered separate bugs on their own which need to be fixed regardless (but sometimes is quite difficult to do so)."

I especially liked dgrunwald's comment:

"A safe mem::forget has the advantage that it makes it easier to write the counterexample proving thread::JoinGuard unsafe. Safe mem::forget makes it more likely that people will know that destructors are not realiable, so they can avoid repeating this mistake."

But then, "Put a big freaking spike in the middle of the steering wheel and get rid of the airbags and seat belts" has always appealed to me as an automotive safety approach.


Yeah, I mean, I guess that leaks in general are possible, which would still let you write `forget`. You're right about that.

But the unsoundness RC bug still relies on unsafe code which was written incorrectly.


I think you need to split the unsoundness bug, which happens because thread::scoped uses unsafe code, from the "you can write mem::forget in safe code" bug, which is arguably not a bug but is at least an enormous footgun waiting to happen.

When the footgun goes off in unsafe code in the standard library, you get use-after-free and memory unsoundness. When the footgun goes off in safe code written by mere humans, you leak arbitrary resources.


The fact that you can write mem::forget in safe code is nothing more than an argument that mem::forget itself should not be marked as unsafe.

The fact remains that as long as you are not using unsafe code yourself, you don't really have to worry about anything. The category of destructors that leave the world in an unsafe state if not run is AFAIK a strict subset of the category of destructors that require unsafe code to be written.


The problem here looks like it is with Rc, which does require unsafe code. I haven't checked, but I don't think the destructors in the thread::scoped issue are unsafe. Even if they are, I can easily imagine another situation where a programmer assuming a safe destructor is always called causes a bug, which is exactly the problem with thread::scoped.

And Rc only looks like it is the problem. Rc is doing exactly what it says it does, as safely as it can. The fact that you can use it to avoid having a destructor called is a side-effect of it doing what it's supposed to do.

If main is safe, mem::forget is safe (remember, Rc is just playing the part of mem::forget), and the destructor for JoinGuard is safe (which it is; I just checked), where is the unsafe code?


`unsafe` is a stateful property, not a local one. The specific invariant here is that you cannot rely on destructors for memory safety, which the old thread::scoped API was doing. The actual unsafe code is actually here: https://github.com/rust-lang/rust/blob/master/src/libstd/thr...

(I myself have been guilty of this same error, of assuming that `unsafe` regardless of its position cannot have far-reaching effects on the overall state of your code.)

The important thing is this: using safe Rust, and using any safe stdlib API, memory unsafety is impossible. This includes stdlib APIs that use `unsafe` internally, such as `Rc`. Proving those interfaces safe is the burden of the Rust developers, and if you can show that there has been an error in these proofs then it is a drop-everything sky-is-falling defcon-1 situation that they must address, up to and including unreleasing the language if no suitable solution came to mind (fortunately, this is quite unlikely).


> where is the unsafe code?

In the thread spawning. The destructor itself isn't unsafe, but it was being used as a guard to enforce the safety of the threading code.

> I can easily imagine another situation where a programmer assuming a safe destructor is always called causes a bug, which is exactly the problem with thread::scoped.

There's a difference between "a bug" and "memory unsafety". If the bug is that the value the destructor guards isn't released (for example, if a File is leaked, the associated file descriptor will never be flushed and closed), then that's not Rust's problem. The programmer probably wanted that file descriptor to be closed, but there's no memory safety violations going on there, and in fact nothing worse than simply stuffing that File into a global array/hashtable and forgetting about it.

The problem with thread::scoped is actually fairly unique. The destructor is not doing much besides just calling join() on the forked thread. The problem is that the JoinGuard object doesn't actually control any access to anything at all, it exists purely as a proxy to represent any values (particularly stack values) that are borrowed by the forked thread. Because it's only a proxy, leaking the guard does not actually make the guarded values inaccessible.

I can't think of any situation besides multithreading that has this same problem. Every other situation involving a RAII object with a lifetime being leaked (and therefore outliving its lifetime, which is technically a violation of borrowck) by definition makes the object inaccessible, which means the data it's protecting cannot be reached. For example, a struct that contains a &-ref to some value may outlive the referenced value if the struct is leaked, but this is still memory-safe because the reference will never be dereferenced. If the reference can be dereferenced, that means code exists that has visibility on the struct, which means the struct hasn't actually been leaked yet, and as long as it hasn't been leaked it's still safe (e.g. because borrowck will still enforce the lifetimes).

Multithreading is special because the forked thread is what's actually accessing the values, without ever going through the RAII object. This means that borrowck cannot guarantee that the RAII object is still alive (and therefore that the "protected" values are still valid). This was considered to be ok as the RAII object would join the thread (therefore ensuring it had finished executing) before destructing, but the ability to leak objects means the join may not happen.

Ultimately, this means that JoinGuard is the moral equivalent of a Mutex that coexists with its protected data rather than owning it (thereby making it possible to write code that accesses the data without holding the lock). It's a bit better than that, because you have to actually go to some effort to leak the JoinGuard as opposed to just doing it accidentally, but the point is that borrowck cannot guarantee that the code that accesses the data does not outlive the guard.

And all of this comes back to my original point, which is that I can't think of another situation where it's actually unsafe to leak the RAII guard (as opposed to merely being a bug) without there being `unsafe` code in the guard itself. Note that the example given for PPYP, Vec::drain_range, uses `unsafe` code in its destructor.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: