diff --git a/Cargo.lock b/Cargo.lock index 8e13f19..f49384d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1350,6 +1350,7 @@ name = "idevice-test-harness" version = "0.1.0" dependencies = [ "idevice", + "jkcli", "tokio", "tracing-subscriber", ] diff --git a/idevice/src/services/afc/file.rs b/idevice/src/services/afc/file.rs index 4b56f7f..d91a2d0 100644 --- a/idevice/src/services/afc/file.rs +++ b/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>>, } +/// 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>, @@ -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, }), } } diff --git a/idevice/src/services/afc/inner_file.rs b/idevice/src/services/afc/inner_file.rs index 7d61603..8770e51 100644 --- a/idevice/src/services/afc/inner_file.rs +++ b/idevice/src/services/afc/inner_file.rs @@ -15,7 +15,7 @@ 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 { (0..n) @@ -23,7 +23,7 @@ fn chunk_number(n: usize, chunk_size: usize) -> impl Iterator { .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,8 +36,6 @@ pub(crate) enum PendingResult { type OwnedBoxFuture = Pin> + 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, @@ -45,10 +43,10 @@ pub(crate) struct InnerFileDescriptor<'a> { pub(crate) pending_fut: Option>>, 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, @@ -56,6 +54,8 @@ pub(crate) struct OwnedInnerFileDescriptor { pub(crate) pending_fut: Option, 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>) -> 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>) -> Result { + self.as_mut().close_inner().await + } + + async fn close_inner(mut self: Pin<&mut Self>) -> Result { 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>) -> AfcClient { - unsafe { Pin::into_inner_unchecked(self).client } + pub fn get_inner_afc(mut self: Pin>) -> 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") diff --git a/tests/Cargo.toml b/tests/Cargo.toml index cfd9021..218a034 100644 --- a/tests/Cargo.toml +++ b/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" } diff --git a/tests/src/afc.rs b/tests/src/afc.rs index 0cad873..a3eb176 100644 --- a/tests/src/afc.rs +++ b/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 diff --git a/tests/src/main.rs b/tests/src/main.rs index 32f8064..468135b 100644 --- a/tests/src/main.rs +++ b/tests/src/main.rs @@ -10,6 +10,7 @@ use idevice::{ lockdown::LockdownClient, usbmuxd::{Connection, UsbmuxdAddr, UsbmuxdConnection}, }; +use jkcli::JkFlag; mod afc; mod amfi; @@ -62,6 +63,11 @@ macro_rules! run_test { async fn main() -> ExitCode { tracing_subscriber::fmt::init(); + let args = jkcli::JkCommand::new() + .with_flag(JkFlag::new("no-warn")) + .collect() + .unwrap(); + println!("idevice test harness"); if std::env::var("RUST_LOG").is_err() { @@ -105,19 +111,24 @@ async fn main() -> ExitCode { ); println!("This test suite can have unintended consequnces on misbehaving code."); println!("Make sure this is a device you are willing to destroy."); - println!("This is your last warning. Continuing in 5..."); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - println!("4..."); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - println!("3..."); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - println!("2..."); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - println!("1..."); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - println!("0..."); - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - println!("Starting test suite on {}", usbmuxd_device.udid); + + if args.has_flag("no-warn") { + println!("Skipping the warn countdown, good luck"); + } else { + println!("This is your last warning. Continuing in 5..."); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + println!("4..."); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + println!("3..."); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + println!("2..."); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + println!("1..."); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + println!("0..."); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + println!("Starting test suite on {}", usbmuxd_device.udid); + } let mut success = 0u32; let mut failure = 0u32;