impl Drop for FileDescriptor/OwnedFileDescriptor

idevice/src/services/afc/file.rs

@@ -12,11 +12,25 @@ use crate::{
     },
 };
 
+/// Handle for an open file on the device.
+///
+/// This handle implements **async `Drop`** to automatically close the file for users who forget,
+/// but only on a **Tokio multi-threaded runtime**
+///
+///
+/// Consider calling `.close()`, it won't drop if expliclity closed
 #[derive(Debug)]
 pub struct FileDescriptor<'a> {
     inner: Pin<Box<InnerFileDescriptor<'a>>>,
 }
 
+/// Owned handle for an open file on the device.
+///
+/// This handle implements **async `Drop`** to automatically close the file for users who forget,
+/// but only on a **Tokio multi-threaded runtime**
+///
+///
+/// Consider calling `.close()`, it won't drop if expliclity closed
 #[derive(Debug)]
 pub struct OwnedFileDescriptor {
     inner: Pin<Box<OwnedInnerFileDescriptor>>,
@@ -36,6 +50,7 @@ impl<'a> FileDescriptor<'a> {
                 path,
                 pending_fut: None,
                 _m: PhantomPinned,
+                dropped: false,
             }),
         }
     }
@@ -60,6 +75,7 @@ impl OwnedFileDescriptor {
                 path,
                 pending_fut: None,
                 _m: PhantomPinned,
+                dropped: false,
             }),
         }
     }

idevice/src/services/afc/inner_file.rs

@@ -15,15 +15,15 @@ use crate::{
 };
 
 /// Maximum transfer size for file operations (1MB)
-const MAX_TRANSFER: u64 = 1024 * 1024; // this is what libimobiledevice uses in it's afcclient
+const MAX_TRANSFER: u64 = 1024 * 1024; // this is what libimobiledevice uses in afcclient
 
 fn chunk_number(n: usize, chunk_size: usize) -> impl Iterator<Item = usize> {
     (0..n)
         .step_by(chunk_size)
         .map(move |i| (n - i).min(chunk_size))
 }
 
-// Used to descripe what the future returns
+/// Descripes what the future returns
 #[derive(Debug)]
 pub(crate) enum PendingResult {
     // writing
@@ -36,26 +36,26 @@ pub(crate) enum PendingResult {
 
 type OwnedBoxFuture = Pin<Box<dyn Future<Output = Result<PendingResult, IdeviceError>> + Send>>;
 
-/// Handle for an open file on the device.
-/// Call close before dropping
 pub(crate) struct InnerFileDescriptor<'a> {
     pub(crate) client: &'a mut AfcClient,
     pub(crate) fd: u64,
     pub(crate) path: String,
 
     pub(crate) pending_fut: Option<BoxFuture<'a, Result<PendingResult, IdeviceError>>>,
     pub(crate) _m: std::marker::PhantomPinned,
+
+    pub(crate) dropped: bool,
 }
 
-/// Handle for an owned open file on the device.
-/// Call close before dropping
 pub(crate) struct OwnedInnerFileDescriptor {
     pub(crate) client: AfcClient,
     pub(crate) fd: u64,
     pub(crate) path: String,
 
     pub(crate) pending_fut: Option<OwnedBoxFuture>,
     pub(crate) _m: std::marker::PhantomPinned,
+
+    pub(crate) dropped: bool,
 }
 
 crate::impl_to_structs!(InnerFileDescriptor<'_>, OwnedInnerFileDescriptor; {
@@ -276,11 +276,17 @@ impl<'a> InnerFileDescriptor<'a> {
 
     /// Closes the file descriptor
     pub async fn close(mut self: Pin<Box<Self>>) -> Result<(), IdeviceError> {
+        self.as_mut().close_inner().await
+    }
+
+    async fn close_inner(mut self: Pin<&mut Self>) -> Result<(), IdeviceError> {
         let header_payload = self.fd.to_le_bytes().to_vec();
 
         self.as_mut()
             .send_packet(AfcOpcode::FileClose, header_payload, Vec::new())
             .await?;
+
+        unsafe { Pin::into_inner_unchecked(self).dropped = true }
         Ok(())
     }
 }
@@ -314,18 +320,36 @@ impl OwnedInnerFileDescriptor {
 
     /// Closes the file descriptor
     pub async fn close(mut self: Pin<Box<Self>>) -> Result<AfcClient, IdeviceError> {
+        self.as_mut().close_inner().await
+    }
+
+    async fn close_inner(mut self: Pin<&mut Self>) -> Result<AfcClient, IdeviceError> {
         let header_payload = self.fd.to_le_bytes().to_vec();
 
         self.as_mut()
             .send_packet(AfcOpcode::FileClose, header_payload, Vec::new())
             .await?;
 
-        // we don't need it to be pinned anymore
-        Ok(unsafe { Pin::into_inner_unchecked(self) }.client)
+        Ok(self.into_inner_afc())
+    }
+
+    fn into_inner_afc(mut self: Pin<&mut Self>) -> AfcClient {
+        let this = unsafe { Pin::into_inner_unchecked(self.as_mut()) };
+
+        this.dropped = true;
+
+        let dummy_afc = AfcClient::new(crate::Idevice::new(
+            Box::new(std::io::Cursor::new(vec![])),
+            "67",
+        ));
+
+        // the `.drop()` won't use the `self.client` if we already dropped it (or don't want to
+        // drop it)
+        std::mem::replace(&mut this.client, dummy_afc)
     }
 
-    pub fn get_inner_afc(self: Pin<Box<Self>>) -> AfcClient {
-        unsafe { Pin::into_inner_unchecked(self).client }
+    pub fn get_inner_afc(mut self: Pin<Box<Self>>) -> AfcClient {
+        self.as_mut().into_inner_afc()
     }
 }
 
@@ -418,6 +442,38 @@ crate::impl_trait_to_structs!(AsyncSeek for InnerFileDescriptor<'_>, OwnedInnerF
     }
 });
 
+crate::impl_trait_to_structs!(Drop for InnerFileDescriptor<'_>, OwnedInnerFileDescriptor; {
+    fn drop(&mut self) {
+        if !self.dropped {
+            // The pending_fut (if Some) holds a Pin<&mut Self> derived from a
+            // raw pointer to this struct. Dropping it here ensures that
+            // mutable reference is released before we create a second one via
+            // Pin::new_unchecked(self) below. Two live &mut Self to the same
+            // struct is UB under Stacked Borrows even if neither is actively
+            // dereferenced.
+            self.pending_fut = None;
+
+            let handle = tokio::runtime::Handle::current();
+
+            if matches!(
+                handle.runtime_flavor(),
+                tokio::runtime::RuntimeFlavor::CurrentThread
+            ) {
+                return;
+            }
+
+            tokio::task::block_in_place(move || {
+                handle.block_on(async move {
+                    unsafe { Pin::new_unchecked(self) }
+                        .close_inner()
+                        .await
+                        .ok();
+                })
+            });
+        }
+    }
+});
+
 impl std::fmt::Debug for InnerFileDescriptor<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("InnerFileDescriptor")

tests/Cargo.toml

@@ -7,3 +7,4 @@ edition = "2024"
 tracing-subscriber = "0.3.23"
 idevice = { path = "../idevice", features = ["full"] }
 tokio = { version = "1", features = ["full"] }
+jkcli = { version = "0.1.1" }

tests/src/afc.rs

@@ -633,7 +633,8 @@ pub async fn run_tests(provider: &dyn IdeviceProvider, success: &mut u32, failur
             for _ in 0..100 {
                 let mut f = client.open(&path, AfcFopenMode::WrOnly).await?;
                 f.write_all(b"hi").await?;
-                // drop f — no explicit close
+                std::mem::forget(f);
+                // file not closed
             }
             // Client must still work after 100 implicit drops
             client.remove(&path).await
@@ -769,7 +770,8 @@ pub async fn run_tests(provider: &dyn IdeviceProvider, success: &mut u32, failur
             {
                 let mut f = client.open(&path, AfcFopenMode::WrOnly).await?;
                 f.write_all(b"written but not closed").await?;
-                // drop f here — the write is complete but FileClose is never sent
+                std::mem::forget(f);
+                // the write is complete but FileClose is never sent
             }
             // AfcClient must still be usable
             let info = client.get_device_info().await?;
@@ -783,6 +785,123 @@ pub async fn run_tests(provider: &dyn IdeviceProvider, success: &mut u32, failur
         }
     );
 
+    // Drop-and-reopen fd-pool exhaustion test: open + drop 200 times relying
+    // entirely on the implicit Drop to send FileClose.  If the unsafe
+    // Pin::new_unchecked Drop path is broken and FileClose is never sent, the
+    // device will exhaust its per-connection fd pool and begin returning errors
+    // before we reach iteration 200.  Each iteration also does a partial read
+    // to ensure pending_fut is populated when Drop fires.
+    run_test!(
+        "afc: drop recycles device fds - 200 open/drop cycles",
+        success,
+        failure,
+        async {
+            let path = p("drop_reopen.bin");
+            roundtrip(&mut client, &path, b"drop reopen test data").await?;
+
+            for i in 0..200u32 {
+                let mut f = client
+                    .open(&path, AfcFopenMode::RdOnly)
+                    .await
+                    .map_err(|e| {
+                        idevice::IdeviceError::UnexpectedResponse(format!(
+                            "open failed at iteration {i} (fd pool exhausted?): {e}"
+                        ))
+                    })?;
+                // Read a byte to exercise the pending_fut state machine before drop
+                let mut buf = [0u8; 1];
+                f.read_exact(&mut buf).await?;
+                // FileDescriptor drops here - Drop impl fires
+                // unsafe { Pin::new_unchecked(self) }.close_inner().await
+                // on a multi-thread tokio runtime, recycling the device fd
+            }
+
+            println!("(200 open/drop cycles succeeded)");
+            client.remove(&path).await
+        }
+    );
+
+    // Stale-fd test: verify that the Drop destructor actually sent FileClose.
+    // Strategy: open a file, record the raw device fd, let it drop, then
+    // reconstruct a FileDescriptor pointing at the *same* raw fd via the
+    // unsafe constructor. Because Drop already sent FileClose the device has
+    // freed that fd; any I/O on the stale descriptor must return an error.
+    // If Drop had NOT fired, the fd would still be open and the read would
+    // succeed - making the test fail.
+    run_test!(
+        "afc: stale fd after drop returns error (proves Drop sent FileClose)",
+        success,
+        failure,
+        async {
+            let path = p("stale_fd.bin");
+            roundtrip(&mut client, &path, b"stale fd sentinel").await?;
+
+            // Open and immediately drop - Drop fires, sending FileClose.
+            let raw_fd = {
+                let f = client.open(&path, AfcFopenMode::RdOnly).await?;
+                #[allow(clippy::let_and_return)]
+                let fd = f.as_raw_fd();
+                // f drops here, destructor runs, device frees the fd
+                fd
+            };
+
+            // Reconstruct a FileDescriptor using the now-closed device fd.
+            // SAFETY (intentionally violated for testing): raw_fd is stale;
+            // we expect every subsequent I/O to return an error.
+            let mut stale = unsafe {
+                idevice::services::afc::file::FileDescriptor::new(&mut client, raw_fd, path.clone())
+            };
+
+            let result = stale.read_entire().await;
+            if result.is_ok() {
+                return Err(idevice::IdeviceError::UnexpectedResponse(
+                    "stale fd read succeeded - Drop may not have sent FileClose".into(),
+                ));
+            }
+            println!("(stale fd correctly returned: {})", result.unwrap_err());
+
+            // Drop stale - it will attempt a second FileClose on an already-closed
+            // fd.  The device will error; Drop discards that error with .ok().
+            // The client must remain usable afterwards.
+            drop(stale);
+
+            client.remove(&path).await
+        }
+    );
+
+    // mem::forget variant: skips the Drop impl entirely (no FileClose sent,
+    // no unsafe Pin path runs).  The device fd leaks but AfcClient must
+    // survive and be fully operational.
+    run_test!(
+        "afc: mem::forget (skip Drop) - client survives, fd leaks",
+        success,
+        failure,
+        async {
+            let path = p("forget_fd.bin");
+            roundtrip(&mut client, &path, b"forget test").await?;
+
+            let leaked_fd = {
+                let mut f = client.open(&path, AfcFopenMode::RdOnly).await?;
+                let raw = f.as_raw_fd();
+                let mut buf = [0u8; 4];
+                f.read_exact(&mut buf).await?;
+                std::mem::forget(f); // destructor never runs - device fd stays open
+                raw
+            };
+            println!("(intentionally leaked device fd={leaked_fd})");
+
+            // Client must still work despite the leaked fd
+            let info = client.get_device_info().await?;
+            if info.model.is_empty() {
+                return Err(idevice::IdeviceError::UnexpectedResponse(
+                    "get_device_info failed after mem::forget".into(),
+                ));
+            }
+            let _ = client.remove(&path).await;
+            Ok(())
+        }
+    );
+
     // ─────────────────────────────────────────────────────────────────────────
     // MULTI-THREADED CONCURRENT TESTS
     // Each test spins up multiple tokio tasks sharing one AfcClient behind an