[HN Gopher] Using Drop for safety in Rust
       ___________________________________________________________________
        
       Using Drop for safety in Rust
        
       Author : thunderbong
       Score  : 100 points
       Date   : 2024-12-16 10:13 UTC (12 hours ago)
        
 (HTM) web link (v5.chriskrycho.com)
 (TXT) w3m dump (v5.chriskrycho.com)
        
       | the_mitsuhiko wrote:
       | The pattern that this relies on is called "pre-pooping" because
       | of this blog post: https://cglab.ca/~abeinges/blah/everyone-
       | poops/
        
         | wizzwizz4 wrote:
         | Canonical URL: https://faultlore.com/blah/everyone-poops/
        
           | cptroot wrote:
           | And here's the more straightforward documentation from the
           | Nomicon that takes the initial blog post and explains it
           | thoroughly and straightforwardly. (It doesn't mention pooping
           | your pants though)
           | 
           | https://doc.rust-lang.org/nightly/nomicon/leaking.html
        
       | sshine wrote:
       | What impresses me with Rust isn't that it has highly efficient
       | and idiomatically written standard library features. It's that
       | the footguns are consistently encapsulated.
        
         | pdpi wrote:
         | One thing that fascinates me about Rust is how, in many ways,
         | the language has become easier over time, because a significant
         | share of the "new" features is shaped like "made an existing
         | construct work across more contexts and smoothed out an
         | irregularity in the language".
        
           | dathinab wrote:
           | yes in general when it comes to rust it tries to make sure
           | that any new abstraction of features don't cause unknown
           | unknowns, at least wrt. safety/soundness.
           | 
           | Like make abstractions only leaky in ways where either the
           | compiler will yell at you if you mess up or it involves
           | unsafe functions with clear definitions about under which
           | conditions they are safe to use. With a strong style bias to
           | not use unsafe.
           | 
           | Like e.g. there are _ton_ of complexity in the exact rules of
           | the borrow checker, but once you know the basics that's most
           | times all you need especially. Or if you don't know async you
           | don't need to know anything about async at all even if you
           | provide a library with closures/callbacks.
           | 
           | and that is lovely, and also where C++ fails (hard IMHO), and
           | where C kinda somewhat doesn't fail (it fails in other places
           | tho IMHO)
        
       | dathinab wrote:
       | TL;DR: YOU MUST NOT
       | 
       | (and as I haven't yet time to read the article I'm not sure if
       | they convey that correctly)
       | 
       | But for various (complicated) reasons Drop isn't guaranteed to
       | run on a "safety" level. That means that means whatever you do
       | you must not rely on Drop for safety.
       | 
       | Through what you can do is rely on it for _correctness_, leading
       | to safe but not very correct outcomes if it's not uphold.
       | 
       | What that means is that while you can (and should) use drop for
       | cleanup patterns with patterns like drop guards, but you need to
       | make sure that if drop isn't run it's still safe, i.e. sound.
       | 
       | It's totally fine to abort the process if a drop which _really_
       | needed to be run isn't run.
       | 
       | One example from `std` would be `vec::Drain` which if dropped
       | leaks all elements of the `Vec` (not just the drained ones) but
       | is safe i.e. sound to leak (also the mem allocation of the vec
       | itself is not leaked just any allocation from elements in the
       | vector).
       | 
       | On exception is if the drop guard itself is part of a unsound
       | block or similar, i.e. if you can limit the scope of unsafe code
       | enough to place the "drop-must" run part into a unsafe contract.
       | E.g. you create drop guard at the beginning of a (non-async)
       | unsafe block and can guarantee as part of the contract for that
       | unsafe block that it's run at the end of it. This is commonly
       | used in compact unsafe code blocks where in go you would use
       | defer.
        
         | wizzwizz4 wrote:
         | This is exactly the example that the article uses.
        
           | dathinab wrote:
           | I know, that was intentional.
           | 
           | It doesn't change that you _must not_ use Drop for safety.
           | 
           | You only can use it for cleanup, and that can involve unsafe
           | code and that can make things easier, but you can't rely on
           | drop running for anything safety i.e. soundness related.
           | 
           | And the article fails to highlight this problem but IMHO you
           | really have to highlight it every time you speak about
           | safety+drop, as its one of the biggest pitfalls of current
           | rust and not doing so is irresponsible. There had been too
           | much unsound code due to people overlooking that drop isn't
           | guaranteed to run.
           | 
           | Like to quote:
           | 
           | > What about after you finish with the draining iterator,
           | though? How does Rust guarantee that part of the contract?
           | 
           | > This is where it gets interesting.
           | 
           | > When the iterator is dropped -- either because you hit the
           | end of a for loop over it or because you drop it after
           | iterating over some subset of elements -- the Drain type's
           | implementation of the Drop trait takes over. That means that
           | impl Drop for Drain is responsible for making sure that Drain
           | is sound.
           | 
           | And that is _wrong_ the Drop impl _must not_ be responsible
           | for making sure Drain is Sound.
           | 
           | The `drain` function call does so by setting len==0, i.e.
           | making the vector forget about all it's content, i.e.
           | semantically moves out all of it's elements. The Drop impl.
           | semantically just moves the parts it didn't drain back into
           | the Vec. I.e. it's isn't responsible for soundness but for
           | not leaking the non drained elements.
        
       | ototot wrote:
       | Haven't read the post, but I think there must be something wrong
       | if a type's safety depends on the Drop trait due to the existence
       | of std::mem::forget.
        
         | wizzwizz4 wrote:
         | That would make the lifetime last forever, preventing you from
         | ever getting the mutable reference back and causing any safety
         | issues. (Your intuition serves you well, though: graphics APIs
         | designed to commit on a guard type's Drop::drop are prone to
         | panicking, since the GPU driver does not care about Rust
         | lifetimes. To implement _that_ properly, you usually need
         | ersatz typestates.)
        
           | ynik wrote:
           | The crucial bit for Vec::drain is in these two lines of code,
           | which the article lists but does not discuss:
           | // set self.vec length's to start, to be safe in case Drain
           | is leaked             self.set_len(start);
           | 
           | You cannot rely on Drop alone for safety, you need a fallback
           | strategy in case of a leak (mem::forget or Rc cycle). Rust
           | lifetimes only ensure that there is no use after free; there
           | is no guarantee that Drop is called before the end of the
           | lifetime. It's possible to mutably access the original Vec
           | after leaking a Drain.
        
             | nextaccountic wrote:
             | Yeah that's the "pre" in "pre-pooping your pants"
        
           | wizzwizz4 wrote:
           | This is misleading. `std::mem::forget(&'a mut v)` does not
           | imply `'a: 'static`.
        
         | Filligree wrote:
         | That is precisely what the post is about.
        
           | dathinab wrote:
           | and fails to properly convey outright stating both in the
           | title and places in the article that drop can be responsible
           | for safety (it can't, in drain it's responsible for not
           | leaking non drained elements, not safety)
        
         | dathinab wrote:
         | yes drop can't be responsible for safety
         | 
         | in Vec::drain() by setting the Vec len to 0, std semantically
         | moves all elements from the Vec to the Drain and in in
         | Drain::drop it semantically moves the non drained elements back
         | 
         | i.e. vec::Drain::drop is responsible for not leaking non
         | drained elements, not for safety
         | 
         | and sure not leaking means positioning them so in the vec that
         | the len can be set to a value where all elements from 0..len
         | have not been moved out etc. but that doesn't change that
         | semantically it's responsibility is "not leaking" instead
         | "making sure it's sound".
        
           | dathinab wrote:
           | some random additional information:
           | 
           | - initially it was believed you can rely Drop for soundness
           | 
           | - then the rust community realized that this is an oversight
           | -- the so called leakocalipyse
           | 
           | - there also was no easy straight forward way to fix that, in
           | the end it was decided that leaking must be sound and
           | `std::mem::forget` was added
           | 
           | - while there was some initial fallout (e.g. scoped threads
           | and some &mut iterators now leaking collections if leaked) it
           | wasn't to bad as you still can use function scopes to enforce
           | some non leaking (i.e. if you accept a closure you can make
           | sure something runs both before and after it)
           | 
           | - but with async functions a lot of the workarounds don't
           | work anymore as the async function could be leaked mid
           | execution by calling async functions ... so by now some
           | people which rust had taken a different turn back then. But
           | then to be fair this is the kind of "hypothetically wishing"
           | because making leak sound was with the knowledge and
           | constraints available back then very clearly the right
           | choice.
        
       | cmiller1 wrote:
       | Not related to the content of this post, but resizing my browser
       | window on that site is kind of trippy.
        
       | dathinab wrote:
       | From the article:
       | 
       | > That means that impl Drop for Drain is responsible for making
       | sure that Drain is sound.
       | 
       | This is _wrong_ the Drop impl _must not_ be responsible for
       | making sure Drain is Sound. Drop is not guaranteed to run it must
       | _never_ be relied on for soundness.
       | 
       | I emphasizes the wrong because that is one of the biggest
       | pitfalls in current rust outside of doing some very clever/magic
       | things.
       | 
       | In std the `drain` function guarantees soundness by setting
       | len=0, i.e. making the vector forget about all it's content, i.e.
       | semantically moves all of it's elements from the Vec into the
       | Drain.
       | 
       | The Drop impl. semantically just moves the parts it didn't drain
       | back into the Vec.
       | 
       | I.e. for Drain Drop it's isn't responsible for soundness but for
       | not leaking the non drained elements.
       | 
       | EDIT: To be fair: Otherwise still a good article. Just misses one
       | crucial paragraph.
        
         | j-krieger wrote:
         | Any link to that magic? Sounds interesting.
        
           | dathinab wrote:
           | The code the blog presents copied from the std, ironically it
           | does include the line where the len is set to 0 with a in
           | code comment (but the article fails to go into it and seems
           | to not be quite aware of the issue, it's otherwise quite
           | good).
           | 
           | Also:
           | 
           | - up to date explanation in the nomicon: https://doc.rust-
           | lang.org/nightly/nomicon/leaking.html
           | 
           | - from 2015 when it was realized leak must be sound:
           | https://faultlore.com/blah/everyone-poops/
        
       ___________________________________________________________________
       (page generated 2024-12-16 23:01 UTC)