[Idea] Attributes to warn if value is dropped without being acted upon

language design
1

There has been a long unresolved issue with types that need some operation to be performed on them before the value is dropped. Examples are:

  1. Handles for external resources where the finalizing operation needs to be performed explicitly, because it may block or produce a result that should be observed and acted upon. See RFC PR #2677 for some recent discussion.
  2. Low-level APIs where an unsafe function must be called on the object before it is dropped. See code in this proposal for an example (I might be doin’ it wrong, though; comments are welcome).

The idea is to add a pair of attributes: one to mark types or function return values similar to must_use, that will cause a compiler warning if the value is dropped without being consumed in a way designated by the other attribute:

#[must_consume = ".close() should be called on the file"]
struct File {
    // ...
}

impl File {
    #[consumes(self)]
    fn close(self) -> io::Result<()> {
        // ...
        Ok(())
    }
}

The consumes attribute suppresses the must_consume lint about that parameter value within the body of the function.

This is trickier for the problem number 2 above, where the value is borrowed (e.g. in a Drop impl). In this case, another function attribute can be used to warn if a function with this attribute is not called by the destructor of a containing type, marking the passed member as consumed:

trait MemHandle {
    // ...
    #[consumes_in_drop(self)]
    unsafe fn release(&mut self);
}

Has anyone considered this solution before? Is it worthy of an RFC?

3

Sounds like you want some weak form of linear types? @Gankra has previously written some stuff on this: https://gankro.github.io/blah/linear-rust/, which goes into detail as to why a strict version of this is sad. However, I think this suffers a lot of the same problems, insofar that you want the compiler to be able to trace the lifetime of an object in an annotated way (i.e., you sort of want a way for functions to say “I not only accept a T, but actually make sure to consume it.”); I don’t see how this proposal addresses that detail in a way that is not, to quote the linked blagpost, deeply unpleasant.

4 Likes
4

Not really; I want a way to say "This function is the correct way to finish usage of T, and if you don't, the program may not do what it's supposed to, or leak resources, while remaining safe in the Rust sense". Similarly to how must_use indicates return values that are important and should be made use of in some way, so immediately dropping the value is a bad code smell, though not a safety hazard. I don't need it to be bulletproof against more elaborate ways to move and dispose of (or fail to dispose of) a value that @Gankra's blog post talked about.

1 Like
5

Embedding of #[must_consume] fields is interesting. A struct with a #[must_consume] field would be obliged to have a #[must_consume]/#[consumes(...)] infrastructure of its own. Dealing with enum values which cannot be statically proven to be, or not to be, of a #[must_consume] variant can prove to be a massive pain in practice, but it can also nudge towards linear/affine type designs such as:

#[must_consume]
struct MayWorkWithFile {
    file: Option<File>,
    stuff: Vec<u8>
}

impl MayWorkWithFile {
    #[consumes(self)]
    fn done(self) -> io::Result<DoneWithFile> {
        self.file.map(|f| f.close()).transpose()?;
        DoneWithFile { stuff: self.stuff }
    }
}
6

My first question would be, why can’t this be solved with a custom Drop (or, after the mentioned RFC, TryDrop) impl that takes care of the required finalization?

The other potentially interesting approach in my mind is to recast this as saying that a type cannot be dropped (through an attribute or marker trait), and then offer a method on the type that takes a self argument and destructures it.

(In general, the latter approach seems like it makes the proposes consumes attribute unnecessary.)

3 Likes
7

Drop cannot really do scary things like blocking because it can be invoked in an unwind. The proposed Close or TryDrop trait would need to be used explicitly and needs a backstop in case the value is mistakenly not finalized through the trait.

As in the case 2 in the OP, the finalizing operation may be unsafe, meaning that the caller has to meet certain guarantees to avoid undefined behavior, which needs to made evident in code by an unsafe block or fn. Doing unsafe things unchecked in Drop is a recipe for disaster.

That can only be practical in abort-on-panic setups, I'm afraid.

8

The relevant point from the blog post is about how this interacts very poorly with various kinds of generics, to the point that many transformations will diliute any value this would otherwise provide. For example, it is unclear how the compiler would appropriately deal with collections or options of #[must_consume] types, which seems like a big hole in the power of this guardrail.

9

The implementation of a generic collection needs to drop elements at some points, which would produce the lint when a #[must_consume] type is used as a parameter. I guess it would be painful to use #[must_consume] types in a general-purpose collection, but on the other hand, the question arises why one would want to do so. If the developers know what they are doing or simply don't care, they can always suppress the lint; it will serve to remind the readers of the code about the potentially improper use.

Options may be painful too, but above I have outlined an example of destructuring an option with some meaningful type conversion design.

10

The calculus is: why do you have such a high-maintenance type that requires such awful ergonomics? If you can't cram your cleanup into a destructor (which is what destructors are for...) I think you have bigger problems that a lint like this one will not save you from.

Phrased differently: if your type requires serious contortions to fit into otherwise general-purpose parts of the language, I suspect you have something that is either fundamentally unsafe (in a way Rust cannot protect you from) or a poor choice of abstraction (your example of "I don't want to make a network call inside of a destructor." indicates to me a very concerning model of resource acquisition).

11

I'd be happy to see a better abstraction for, e.g., std::fs::File. Or for my memory handle design of a custom allocation layer meant to be built upon by containers, that requires the caller to assert that use of the allocated buffer has really ceased at the point where the handle is released. I myself love my perfectly ergonomical RAII types when I can have them, but some of us have to deal with mean and unforgiving system stuff...

As long as it's not UB, I expect to be able to do it in Rust with the best ergonomics it can provide.

12

Maybe the proposed semantics could be "only use Drop in case of a panic!(), otherwise don't allow Drop -- the author must somehow destructure the type", then?

13

I don't get this. In the case of an unwind, you want to just leak your resources rather than attempt to close them gracefully? This seems wrong to me.

(If the argument is just "drop should be 'fast', such that unwinding is 'fast'", a) unwinding is programmer error and doesn't need to be fast, it's "kill this thread" level errors only, and b) that ship has already sailed with major libraries like rayon and crossbeam using drop points for synchronization.)

I think what you would benefit from is a Drop that does attempt to finalize the resource with a handling of any errors (probably panicking or logging), and a by-value finalize that returns the error and mem::forgets to run the standard Drop.

For example:

struct RawHandle { .. }

pub struct Handle {
    raw: ManuallyDrop<RawHandle>,
}

impl Handle {
    fn close(mut self) -> Result<_, _> {
        let mut raw = ManuallyDrop::take(&mut self.raw); // still unstable
        mem::forget(self);
        // UFCS for clarity
        RawHandle::close(&mut raw)
    }
}

impl Drop for Handle {
    fn drop(&mut self) {
        RawHandle::close(&mut self.raw).map(drop).unwrap_or_else(|e| {
            if panicking() {
                log_error!("Failed to close handle during unwind; attempting to continue: {}", e);
            } else {
                panic!("Failed to close handle: {}", e);
            }
        });
    }
}
14

How is the RAII abstraction not good enough? Other languages basically add discount RAII (see, Java's try-with-resources, Python's with, etc) to deal with automatically closing fds.

I don't agree that "system stuff" is mean and unforgiving-- I have found that RAII is a wonderful thing to have when writing ring0 code.

2 Likes
15

Not necessarily, though "graceful" attempts should rather not try to affect the program state too much. Your example does more or less the right thing, except that log_error! should be implemented very carefully to not abort the program, and a panic on error may be not what the user wants to happen: it's also very common and legitimate to not care about close errors. A user-friendly way to solve this is to steer the API consumer into calling Handle::close and deciding what to do with the result. So there should be a way to discourage them from closing handles implicitly by dropping them.

Perhaps the emphasis on "fast" is wrong here. In the main, there is a difference between "kill this thread" and "block this thread waiting for things to happen outside of the program before it can be killed", that I'm not comfortable with.

That's legitimate if the synchronization is internal to the program (or, better, the specific library putting safe abstractions around it) and is expected to be resolved fast, assuming no deadlocks.

16

So what the Rust equivalent for that could look like? Numerous issues on The Rust Programming Language ¡ GitHub have been filed and closed or postponed trying to offer such a language mechanism. The blog post you have kindly referenced earlier in this thread suggests that a strict form of enforcement would be both painful in practical use and not reliable enough to justify its strictness.

17

Expanding on how this could work with generic collections, dropping a Vec<Handle> would result in a drops_must_consume lint because the implementation of Drop for Vec uses ptr::drop_in_place. It would be nice if this did not: vec.into_iter().for_each(|h| { let _ = h.close(); }); This, however, can’t be proven at compile time to not drop items outside of Handle::close, and indeed if a call to the latter panics, the remaining items will be dropped.

18

Is "things outside of the program" directed at stuff like network operations, or stuff like (non-network) system calls? The running example has been closing files, which seems like (non-network) system calls, but I usually consider the operating system to be not "outside" the program so much as "underneath" it, like the Rust language and the memory allocator and the CPU and so on.

2 Likes
19

To clarify: the feature other languages go to lots of effort to sort-of add to deal with files are dtor-like mechanisms to implement something-like-RAII. RAII is a first-class feature in Rust that drives many of its abstractions (and, frankly, the only reasonable way to deal with resource management).

1 Like
20

In both examples you cited the language runtime has exception handling facilities to gracefully deal with the situation when resource-finalizing code fails. What could be the Rust equivalent for that?

21

Rust does not have an exception-handling mechanism, full stop, so this question does not make sense. Those other languages crammed RAII into their exception-handling syntax because they had finally, which serves a similar purpose. Rust does not have (or need, because we have RAII) finally.

I think this thread has derailed, since I think we’re very far away from anything having to do with linear types.

1 Like