Should or should you not block on drop in tokio?

help
1

For types which use async functions and implement Drop, there's an inherent issue of doing async work inside the drop implementation.

Consider this code:

struct AsyncStruct(Arc<RwLock<u32>>);

impl Drop for AsyncStruct {
     fn drop(&mut self) {
           *self.0.blocking_write() = 0
     }
}

Or this code:

impl Drop for AsyncStruct {
     fn drop(&mut self) {
           let lock = self.0.clone();
           tokio::spawn(async move {
               *lock.write().await = 0;
           });
     }
}

Both are valid- the first blocks the worker thread until it can drop, and the other
spawns a task to do the drop. I'm not sure what the trade-offs are, and which is the correct choice- for example, tokio::fs::file uses the sync close on drop (and does not spawn a task to close).

2

Generally, the rule about blocking is that you must not spend a long time without reaching an .await. Destructors are unable to use .await, which means that they must not take a long time.

You give one example of tokio::fs::File. It's true that it closes the file synchronously. However, this operation does not take very long so it's okay.

In other places within Tokio, you will see use of std::sync::Mutex in destructors. In those cases, it's okay because everyone who locks the mutex does so for a short time, so locking it in the destructor will not have to wait a long time for the mutex. Since the destructor doesn't take a long time, this is okay even though locking a mutex could be called "blocking".

However, you are using tokio::sync::RwLock. The advantage of tokio::sync::RwLock over std::sync::RwLock is that the Tokio lock can be held locked over an await, which means that it can be held locked for a long time. This means that it's not okay to lock it in your destructor.

(As an aside, calling blocking_write will result in a panic because Tokio tries to detect such bugs for you.)

You can spawn a task. You can also consider using std::sync::RwLock instead if this is appropriate for your use-case.

4 Likes
3

That isn't always true. Consider writing a large file to a slow external media (maybe a SD card or USB drive) that is mounted as sync, then closing a file can take a long a long time (on some OS, I seem to remember at least Windows used to do this back when I used it last, which was many years ago).

Network file systems such as NFS can also throw curve balls.

1 Like
4

NFS is problematic in sync Rust as well because close can fail, but AFAICT the file API doesn't expose the error.

5

Any use of shared storage hardware could cause long latencies, or at least unpredictable ones, for sync on close.

Since tokio file handles are closed synchronously on drop, a work around is to do an explicit sync_data or sync_all before dropping the handle.

6

That makes sense, and you should probably think about this by default as you don't know where the end user may be saving the file or running the program from. Could well be on a network share or other problematic media.

As such the statement made above that files are quick to close is a dangerous "default answer" without understanding the context the code will be used in. Not everything is about web 5.0[1] servers deployed on hyperscaler hardware that you control.

  1. Or whatever buzzword marketing number they reached by now, I stopped keeping track. ↩ī¸Ž

7

using something like a drop bomb or static analysis lints could be used to lint for this, using some sort of async destructors instead, could be a way to avoid this in a codebase.

8

Here is a discussion about async drop (just put something related).

2 Likes