fs(tests): do not leak FDs with shutdown background (#8184)

---------

Co-authored-by: Martin Grigorov <[email protected]>
Co-authored-by: Mattia Pitossi <[email protected]>

tokio/tests/fs_uring_cancel_open.rs

@@ -15,7 +15,7 @@ use std::task::Poll;
 use tempfile::NamedTempFile;
 use tokio_test::assert_pending;
 
-use crate::support::io_uring::io_uring_supported;
+use crate::support::io_uring::{assert_fds_are_not_leaking, io_uring_supported};
 
 mod support {
     pub(crate) mod io_uring;
@@ -30,10 +30,10 @@ async fn file_descriptors_are_closed_when_cancelling_open_op() {
 
     let tmp = NamedTempFile::new().unwrap();
     let path = tmp.path().to_path_buf();
-
+    let file_number = 128;
     let fd_count_before_opens = fs::read_dir("/proc/self/fd").unwrap().count();
 
-    for _ in 0..128 {
+    for _ in 0..file_number {
         let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
 
         let path = path.clone();
@@ -67,14 +67,5 @@ async fn file_descriptors_are_closed_when_cancelling_open_op() {
         assert!(res.is_cancelled());
     }
 
-    let fd_count_after_cancel = fs::read_dir("/proc/self/fd").unwrap().count();
-    let leaked = fd_count_after_cancel.saturating_sub(fd_count_before_opens);
-
-    // Since we are opening 128 files, we expect that the related fds
-    // related to this operation will be closed. Since some other fds
-    // can be opened in the meantime, we expect this number to be higher
-    // than the counter before opening the files. This number could be
-    // lower, but to avoid test flakiness we check that this is at most
-    // half the number of the file we opened to check if there's a leak.
-    assert!(leaked <= 64);
+    assert_fds_are_not_leaking(fd_count_before_opens, file_number, 1).await
 }

tokio/tests/fs_uring_completed_then_dropped_before_repoll.rs

@@ -23,7 +23,7 @@ use tokio::sync::mpsc::{unbounded_channel, UnboundedSender};
 use tokio::time::timeout;
 use tokio_test::assert_pending;
 
-use crate::support::io_uring::io_uring_supported;
+use crate::support::io_uring::{assert_fds_are_not_leaking, io_uring_supported};
 
 /// Count currently-open fds in this process.
 fn fd_count() -> usize {
@@ -123,23 +123,12 @@ fn uring_completed_then_dropped() {
         let before = fd_count();
         let tmp = NamedTempFile::new().unwrap();
         let path = tmp.path().to_path_buf();
+        let file_number = 128;
 
-        for _ in 0..128 {
+        for _ in 0..file_number {
             completed_then_dropped_before_repoll(path.clone()).await;
         }
 
-        // Give completions a moment to settle before counting fds.
-        tokio::time::sleep(Duration::from_millis(250)).await;
-
-        let after = fd_count();
-        let leaked = after.saturating_sub(before);
-
-        // Since we are opening 128 files, we expect that the related fds
-        // related to this operation will be closed. Since some other fds
-        // can be opened in the meantime, we expect this number to be higher
-        // than the counter before opening the files. This number could be
-        // lower, but to avoid test flakiness we check that this is at most
-        // half the number of the file we opened to check if there's a leak.
-        assert!(leaked <= 64);
+        assert_fds_are_not_leaking(before, file_number, 1).await
     });
 }

tokio/tests/fs_uring_runtime_shutdown.rs

@@ -12,6 +12,7 @@ use futures::FutureExt;
 use std::fs;
 use std::future::poll_fn;
 use std::task::Poll;
+use std::time::{Duration, Instant};
 use tempfile::NamedTempFile;
 use tokio::runtime::Builder;
 use tokio_test::assert_pending;
@@ -74,14 +75,22 @@ fn shutdown_runtime_while_performing_io_uring_ops() {
 
     rt.shutdown_background();
 
-    let fd_count_after_cancel = fs::read_dir("/proc/self/fd").unwrap().count();
-    let leaked = fd_count_after_cancel.saturating_sub(fd_count_before_opens);
+    let fd_check_start = Instant::now();
 
-    // Since we are opening 128 files, we expect that the related fds
-    // related to this operation will be closed. Since some other fds
-    // can be opened in the meantime, we expect this number to be higher
-    // than the counter before opening the files. This number could be
-    // lower, but to avoid test flakiness we check that this is at most
-    // half the number of the file we opened to check if there's a leak.
-    assert!(leaked <= 64);
+    while fd_check_start.elapsed() < Duration::from_secs(1) {
+        let fd_count_after_cancel = fs::read_dir("/proc/self/fd").unwrap().count();
+        let leaked = fd_count_after_cancel.saturating_sub(fd_count_before_opens);
+
+        // Since we are opening 128 files, we expect that the related fds
+        // related to this operation will be closed. Since some other fds
+        // can be opened in the meantime, we expect this number to be higher
+        // than the counter before opening the files. This number could be
+        // lower, but to avoid test flakiness we check that this is at most
+        // half the number of the file we opened to check if there's a leak.
+        if leaked <= 64 {
+            // test success
+            return;
+        }
+    }
+    panic!("Number of FDs is staying above 64. There is probably an FD leak.");
 }

tokio/tests/fs_uring_statx_fd_leak_test.rs

@@ -24,7 +24,7 @@ use tokio::sync::mpsc::{unbounded_channel, UnboundedSender};
 use tokio::time::timeout;
 use tokio_test::assert_pending;
 
-use crate::support::io_uring::io_uring_supported;
+use crate::support::io_uring::{assert_fds_are_not_leaking, io_uring_supported};
 
 /// Count currently-open fds in this process.
 fn fd_count() -> usize {
@@ -139,23 +139,12 @@ fn uring_completed_then_dropped() {
         let before = fd_count();
         let tmp = NamedTempFile::new().unwrap();
         let path = tmp.path().to_path_buf();
+        let file_number = 128;
 
-        for _ in 0..128 {
+        for _ in 0..file_number {
             completed_then_dropped_before_repoll(path.clone()).await;
         }
 
-        // Give completions a moment to settle before counting fds.
-        tokio::time::sleep(Duration::from_millis(250)).await;
-
-        let after = fd_count();
-        let leaked = after.saturating_sub(before);
-
-        // Since we are opening 128 files, we expect that the related fds
-        // related to this operation will be closed. Since some other fds
-        // can be opened in the meantime, we expect this number to be higher
-        // than the counter before opening the files. This number could be
-        // lower, but to avoid test flakiness we check that this is at most
-        // half the number of the file we opened to check if there's a leak.
-        assert!(leaked <= 64);
+        assert_fds_are_not_leaking(before, file_number, 1).await
     });
 }

tokio/tests/support/io_uring.rs

@@ -6,6 +6,11 @@
     target_os = "linux"
 ))]
 
+use std::{
+    fs,
+    time::{Duration, Instant},
+};
+
 use io_uring::IoUring;
 
 // Currently, we are running some of the tests on Kernels where io_uring is not supported
@@ -22,3 +27,29 @@ pub fn io_uring_supported() -> bool {
         ),
     }
 }
+
+#[allow(dead_code)]
+pub async fn assert_fds_are_not_leaking(count_before: usize, opened_files: usize, timeout: u64) {
+    let fd_check_start = Instant::now();
+
+    let max_leaked_fd = opened_files / 2;
+
+    while fd_check_start.elapsed() < Duration::from_secs(timeout) {
+        tokio::task::yield_now().await;
+
+        let fd_count_after_cancel = fs::read_dir("/proc/self/fd").unwrap().count();
+        let leaked = fd_count_after_cancel.saturating_sub(count_before);
+
+        // Since we are opening {opened_files} files, we expect that the related fds
+        // related to this operation will be closed. Since some other fds
+        // can be opened in the meantime, we expect this number to be higher
+        // than the counter before opening the files. This number could be
+        // lower, but to avoid test flakiness we check that this is at most
+        // half the number of the file we opened to check if there's a leak.
+        if leaked <= max_leaked_fd {
+            // test success
+            return;
+        }
+    }
+    panic!("Number of FDs is staying above {max_leaked_fd}. There is probably an FD leak.");
+}