Tokio: Dropping a File does not close it

[eminence]

Version

tokio v0.2.13
tokio-macros v0.2.5
rust 1.42.0-nightly

Platform

Linux 64-bit Debian, kernel 4.19.0-5-amd64

Description

I'm trying to write a file to disk, and then execute it. To do both, I'm using tokio::fs::File and tokio::process::Command and the default executor. Here's my code:

use std::os::unix::fs::OpenOptionsExt;
use tokio::io::AsyncWriteExt;

#[tokio::main]
async fn main() {

    let mut std_options = std::fs::OpenOptions::new();
    std_options.create(true).truncate(true).write(true).mode(0o750);
    let tokio_options = tokio::fs::OpenOptions::from(std_options);

    let mut file = tokio_options.open("/tmp/foo.sh").await.unwrap();
    file.write_all("#!/bin/sh\necho hello".as_bytes()).await.unwrap();
    drop(file); // drop file to close it

    let status = tokio::process::Command::new("/tmp/foo.sh").status().await.unwrap(); // line 15
    println!("Status: {:?}", status);
}

The docs here say that dropping a File will close it (which is why you see me trying to do that in the above code). When I run this code, it sometimes works, but most of the time it fails with this error:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 26, kind: Other, message: "Text file busy" }', src/main.rs:15:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I think this means that my program still /tmp/foo.sh open when tokio::process::Command tries to execute it. This is surprising to me, since the docs suggest that dropping a file is how you close it (and I did not see any explicit "close" methods).

Am I missing something? How can I safely close this file so that it can be used?

Thanks!

[udoprog]

Hey,

So the short version is that you should flush the file to make sure that all pending operations are completed at a specific point in your code, so adding the following should make it work as expected:

file.write_all("#!/bin/sh\necho hello".as_bytes()).await.unwrap();
file.flush().await.unwrap(); // <- add this after writing

The slightly longer version is that all filesystem operations are performed on a separate threadpool (accessed through spawn_blocking). This keeps the file handle alive while there are pending operations in the background even though the tokio File instance you created is dropped. A way that we wait for these pending operations to be applied for the File instance, is via AsyncWriteExt::flush.

This does sound like a good documentation item to fix.

[eminence]

Thanks! This did the trick.

As for documentation, something like the following would have helped me:

Files are automatically closed when they go out of scope, but you should always call flush() to ensure changes are written to disk

Does that sound like the right thing to say? (I can open a PR for that change).

[simonbuchan]

The essential problem here is that Drop isn't async, right? Could tokio offer something like:

trait AsyncDrop {
  type Drop: Future<()>;
  fn async_drop(self) -> Self::Drop;
}

...
file.async_drop().await;

To contractually ensure the file is closed after that line?

[Darksonn]

@simonbuchan You can use flush() for that.

[carllerche]

Neither flush nor drop will sync data to disk. This is true for std as well. One of the sync functions must be called.

[eminence]

But you must sync + drop a file before using it, correct? Would you accept a documentation change that makes this more clear to users?

[Darksonn]

I am working on a bigger doc change that addresses this (among other things).

[carllerche]

Well, i should probably revise my statement.

When you close a file, it will get synced, but errors are not reported and I don't think there are guarantees on when the sync happens.

This should also be true of Tokio. If you await on a write, drop the file, the write should be persisted to disk eventually unless an error happens. If this is not true, then there is a bug.

[eminence]

Yes, if I wait some time after dropping a file, the file gets closed and I see the changes on disk. But would be nice to not have to add arbitrary delay_for calls in my code :)

I also confirm that sync_all + drop is working for me (but both are required -- omitting either causes problems), and I'm looking forward to the doc improvements from @Darksonn

[Darksonn]

If I have misunderstood some detail regarding the syncing business in the PR, please leave a review that says so. I have committed this particular doc change separately, so you can view it here.

[simonbuchan]

@simonbuchan You can use flush() for that.

Are you forgetting Windows' exclusive file modes? Closing is a separate operation from flushing data. There's probably situations on Unix where you might care about it too (free space?)

[simonbuchan]

Ah, reading the new text is a bit clearer what you meant, but the API is still not particularly intuitive. A method that takes the file rather than borrowing would be much less confusing.

[Darksonn]

Tokio's file IO works by running a blocking file operation in a separate thread, and part of what flush does is to wait for that thread to finish. If there are no currently running operations, dropping the file should immediately close the file handle. If you needed more than just closing the file handle (e.g. fsync?), then you would have to call that function separately.

Are you forgetting Windows' exclusive file modes?

I don't even know what those are.

[Darksonn]

There are definitely some details on file systems that I am not familiar with, and if you can see any mistakes in the paragraph that just got merged, I'd love to know about them.

[simonbuchan]

On Windows, you specify if a file can be opened by other processes for reading it writing while you have it open. Generally this works out as while you have a file open for writing, no one else can open it. The point at which you close the file is really really important, nothing to do with when the data hits disk.