Question: closing resources (like std::io::Write)

libs
1

I guess I'm not the first one who was thinking about following problem, but I didn't find a good solution, so here it comes.

Let's consider a simple code snippet:

let mut file = std::fs::File::create("test")?;
file.write(&b"hello"[..])?;

What happens when file is dropped? Well, the documentation tells that all errors, which may happen during the operation, are silently ignored and sync_all should be used before dropping it if the errors must be handled.

Let's go a level up. What if I have a generic method that uses std::io::Write instead? Judging from the approach that BufWriter takes, it is a good idea to call flush explictly before letting it drop, but it is just a desperate attempt to save the day, there is nothing to rely on.

We're closing to the point which led me to this post. I implemented a codec for bitwise compression (a task from Rosetta Code actually). The implementation provide a stream-like interface, which is convenient for some use cases, but I wanted to provide yet adapters for Read and Write, which would work better for files etc. With Write I got stuck. The last byte of the bit stream may need padding, but the padding must be emitted only when the stream ends. Where it should be emitted?

  • It can't be emitted in flush, because it would corrupt the data unless flush was called only just before dropping the the output. So, flush for such an adapter never flushes really everything.
  • The right place to emit the padding would be the close operation. But which? If I make an implementation-specific one, it would be useless for generic implementations working with just Write (which is one of the points mentioned above).
  • If emitting the padding is postponed to drop and the operation fails, I mean the underlying Write::write fails, the client code never learns that the written data are incomplete (and thus corrupted). And implementing it in drop seems tricky to me anyway – I'd rather avoid it.

In general, I consider RAII really fine and matching most use cases well, but I have strong doubts whether it is good enough for the cases when the destructor should execute an operation that might fail and the error should be known to the caller.

What is the best practice and recommendation here? From the little what I saw, I'm not sure if there is really an established general way to cope with that.

Just to write down my current thoughts, I share a bold suggestion. (Well, my background is Java – you were warned –, therefore the similarity with Java's Closeable is not a coincidence.)

Would not be worth having a trait like this TryDrop (I considered naming it Close, which suits better the described case, but I think it might have more general uses):

trait TryDrop {
    type Result;
    fn try_drop(self) -> Self::Result;
}

Such a trait would provide a common way to dispose resources, which might fail, and still indicate the problem. I guess it could be possible to provide some sensible default implementations, e.g., for Write (to flush before drop) and other types like BufReader or File might provide even better variant that leverages their specifics. For instance:

impl TryDrop for File {
    type Result = std::io::Result<()>;
    
    fn try_drop(self) -> Self::Result {
        self.sync_all()
    }
}

Further I guess that it would be nice to have Clippy warning when a type implementing this trait is simply dropped. The warning should point out that any failures of the destructor would be ignored and it is recommended to use the method of this trait to dispose the resource and handle the error safely.

3 Likes
2

Most of the past discussion around this calls it "linear types" and it would be really nice. You can fake it in several ways (e.g. drop_bomb or by making drop unlinkable), and this typically doesn't 100% work.

2 Likes
3

I think you're looking for

2 Likes
4

My idea for your problem (without TryDrop): make sure you don't pass ownership of your type to generic code e.g. don't impl Write for YourType, but only impl<'a> Write for &'a mut YourType.

It means generic code handling impl Write cannot drop your type and thus no errors are ignored. And your non-generic code can handle the close operation explicitly.

3 Likes
5

Could you split the resource into two parts?

From RAII-perspective, you could use an "outer" and "inner" resource. In your example, the inner could be std::io::Write. Some error on drop of the inner could be flagged in the outer which can then be checked.

6

Just an idea:

trait TryDrop {
    type Error;
    fn try_drop(&mut self) -> Result<(), Self::Error>;
}

and a T that implements TryDrop can only be dropped in a scope that returns Result<T, U> where U: From<<T as TryDrop>::Error>

so code like this:

impl TryDrop for File {
    /* -snip- */
}

fn main() -> Result<(), Box<dyn Error>> {
    let file = File::open("foo")?;
    // do something with file
    Ok(())
}

is converted to code like this

/* -snip- */

fn main() -> Result<(), Box<dyn Error>> {
    let file = File::open("foo")?;
    // do something with file
    
   file.try_drop()?;
    
    Ok(())
}

similarly how Drop works (sort of)

7

Consider not dropping the writer. Various Write wrappers have into_inner that allows getting the original file handle back, and close it carefully if needed.

8

Yes! That's basically it, perhaps with some small differences, but the idea is the same. It seems I'm not the first one indeed :slight_smile: Thank you for the link.

I'll try to comment the other replies as well and it seems to me that they mostly miss the point. So I'd like to emphasize a few concerns:

  • The first big question: how to deal with destructors that should perform an operation that may fail and how to not let the failure pass silently?
  • The second big question: how to make such a kind of destructors composable?

The particular example with the bitwise I/O just shows a prominent example where std::io::Write alone lacks something that would make implementation of better, more powerful and more generic decorators possible. And I wondered if I missed something which would make my suggestion irrelevant.

It is a kind of workaround. It is applicable in some scenarios, but actually answers none of the big questions. How much impractical it could be? Just imagine BufWriter implemented in that way.

I think I don't get it. And it sounds like something actually more and unnecessarily complex than the suggestion with TryDrop. I could be wrong though – an example, please?

I see the only significant difference from my TryDrop suggestion that yours takes &mut self. But this is a very important difference, because it does not prevent using the "tried to drop" instance, which means that it has to mark and check its invalid inner state. Which I would like to prevent – and that's why I suggest taking self instead. (Btw. the error type could possibly even return self alongside with the error detail in the case of recoverable errors… and if the client does not care, it can be simply dropped hard then.)

Yes, I considered it. It is basically similar to @pcpthm's suggestion, right? So I don't see it as a real solution. Both have one common property: they put the burden on the client of the code and force the client to take care of details that the composed type should be able to handle on its own.

To make it more explicit – assuming Write: TryDrop<Result = std::io::Result>, we could write:

fn write_file<W: Write>(mut f: W) -> std::io::Result<()> {
    f.write_all(&b"Hello"[..])?;
    f.try_drop()?
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    if let Err(e) = write_file(MyCompressingWriter::from(File::create("test.bin")?)) {
        println!("File corrupted, because an error occurred: {}", e);
    }

    Ok(())
}

In this simple case, a first attempt to use some into_inner which performs the last write of the decorator and returns the underlying Write implementation:

fn write_file<W: Write>(f: &mut W) -> std::io::Result<()> {
    f.write_all(&b"Hello"[..])?;
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut f = MyCompressingWriter::from(File::create("test.bin")?);
    write_file(&mut f)?; // Ehm, we should handle it too, right?
    let f = f.into_inner()?; // And again…
    f.sync_all()?; // And again… I suppose another function should wrap it all
    Ok(())
}

And yet the code must know that there is something like into_inner specific for MyCompressingWriter.

Obviously, write_file stands here for anything arbitrarily complex which takes the ownership of the resource to write some data in. I guess that in such a case the workaround will not work anymore.

9

This is still far from "perfect", though: consider the case where f.write_all fails. Then you never get to call f.try_drop, and call the normal drop handler.

3 Likes
10

Good point :+1:

However, even if write_file just borrowed the resource and dropping it was handled by the caller, the calling code would be simpler and could be generic (i.e., not depending on a type-specific stuff like MyCompressingWriter::into_inner). And that would be important if that would be a method of another type that owns some Write and is concerned just by using it and closing it cleanly.

11

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.