-
Notifications
You must be signed in to change notification settings - Fork 267
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
Document when we *do* guarantee that drop runs #135
Comments
I would especially appreciate if someone could properly write down in one of our primary sources (nomicon, trpl, or reference) what on earth happens with double-panics and panic-in-destructors. I feel like I've always received vague hand-waves on this matter. |
Note the nomicon discusses some stuff:
But these docs are getting extremely long in the tooth (ManuallyDrop exists now, for instance). |
That would be rust-lang/reference#348. I see as as somewhat orthogonal to whether unsafe code is allowed to do things like |
The most important thing, imo, is that the "fixed" scoped_thread pattern works: fn main() {
let state_to_get_borrowed_by_other_thread = ...;
scope(|spawner| {
for i in 0..10 {
let borrow = &state_to_get_borrowed_by_other_thread;
spawner.scoped_thread(move || {
process(borrow);
});
}
);
// if we get here, it must be safe to do this
drop(state_to_get_borrowed_by_other_thread);
}
// can't be bothered to work out the lifetimes here
fn scope(func: impl FnOnce(SpawnerHandle)) {
// has a destructor that ABSOLUTELY must have run
// IF its borrows expire AND the thing it borrowed is still accessible.
// Created in this closure so user can't interfere with this.
let spawner = Spawner::new();
func(SpawnerHandle(&mut spawner));
}
impl Drop for Spawner {
fn drop(&mut self) {
block_on_all_threads();
}
} I expect this means you cannot allow longjmp or killing a thread without unwinding. Although I'm too sick to think about this clearly. |
Arguably you can still "allow" this along similar lines of how I suggested permitting double-drop: implementors of Drop that are properly kept in local vars are guaranteed that their destructor will run if their var goes out of scope, but it is not language-level UB to violate this expectation. Library code is allowed to assume this doesn't happen and can cause UB if it is "skipped" though. It is up to the user of [anything like longjmp] to ensure they are only jumping over safely leakable code. Since no one will bother to document this, this basically means only jumping over your own/audited code. |
I agree, though it seems both rayon and crossbeam implement that with
Absolutely, I meant "guarantee" as in "as part of the contract between a safe library and its clients", not as in "UB". |
I don't think catch_unwind is notably different from a destructor in this case? |
I don't think so either. |
As one example for what this guarantee enables, I claim (if I made no mistake) that this is a safe abstraction (idea and name shamelessly stolen from this crate): #![no_std]
use core::mem;
/// Temporarily move the `T` out of `x`, and pass it through `f`.
/// If `f` returns, write the result back into `x`.
/// If `f` panics, write `fallback` into `x` (or abort if `fallback` is `None`).
pub fn take_mut<T>(x: &mut T, f: impl FnOnce(T) -> T, fallback: Option<T>) {
// A drop guard to implement the fallback behavior.
struct Guard<T> { ptr: *mut T, fallback: Option<T> }
impl<T> Drop for Guard<T> {
fn drop(&mut self) {
// This will double-panic and hence abort if there is no fallback.
let fallback = self.fallback.take().unwrap();
unsafe { self.ptr.write(fallback); }
}
}
let ptr = x as *mut T;
let guard = Guard { ptr, fallback };
// Do the thing!
unsafe { ptr.write(f(ptr.read())); }
// If we got here, there was no panic. Discharge the guard.
mem::forget(guard);
} |
@gankro pointed out that (if I am reading this correctly) hashbrown's |
no its for binary_heap’s heapify operation (in case cmp panics) |
oops sorry, fixed. |
While reading the last sentence, it came to me that on some OSes, it is possible to kill a thread outside the process, such as Windows. As already noted, any function can decide to For scoped threads, the idea of sharing a part of memory from the current thread's stack for use by another thread already sounds a bit odd (googling turns up https://software.intel.com/en-us/inspector-user-guide-windows-cross-thread-stack-access; it does not prohibit it but looks like it's not recommended either). This is complicated by the fact that thread stacks are often managed by the OS and has its own semantics. The thread is always free to use its own stack, but whether it is sound to share it to another thread may depend on the OS. For example, if the thread is killed outside the process, the stack may be freed and thus cause UB. It's usually not expected but it's worth noting. It might be clearer in semantics to guarantee that if |
This is comparable with using Also see rust-lang/unsafe-code-guidelines#211. |
Yep. I’m not a Windows expert, but to quote MSDN on the API that would be used to kill a thread:
(The list keeps going. Elsewhere in the article it mentions that the thread stack is freed.) So even though the API exists, it doesn’t seem like users are expected to go around manually killing threads, like they can do with processes. Indeed, as far as I can tell, the OS doesn’t have any built-in application or command capable of doing it. Process Explorer can do it, and it is owned by Microsoft these days, but it’s very much a “power user” type of tool. |
In my understanding, we do in some circumstances guarantee that drop runs. For example:
Here, we guarantee that no matter the environment or whatever
f
does, if the stack frame oftest
ever gets popped or otherwise "deallocated", then theprintln!
certainly happens. For example,f
might loop forever or abort the process, but it cannot "return" or "unwind" or finish in any other way that would circumvent the printing, nor can it uselongjmp
to skiptest
's cleanup, not can it just ask the OS to outright kill the current thread (without tearing down the entire process).(By "guarantee" I mean "we consider it okay for safe libraries to rely on this and cause UB if it gets broken" -- but there is no immediate language-level UB caused by this, so if you do this kind of skipping of destructors in a controlled way, say for your own code which you knows has nothing droppable on the stack, then you are fine.)
This is needed to actually realize the pinning drop guarantee, but it seems not to be documented anywhere explicitly?
Cc @gankro @nikomatsakis @comex
The text was updated successfully, but these errors were encountered: