From 25f19cb850213d10257ebb45f21931e50f6a9f19 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Wed, 16 Jul 2025 11:23:04 +0000 Subject: [PATCH] ANDROID: rust_binder: reorganize process cleanup This fixes some test failures in rustBinderTest caused by commit f06dd0cd3562 ("ANDROID: rust_binder: release threads before refs"). Honestly I'm not entirely sure why. Note that this reverses that commit, and fixes the resulting logspam by instead moving cleanup of allocated buffers to the end of process release. Bug: 431914626 Change-Id: I1f7faa486f86cef7fbe37ad87b720573a5e05ee5 Signed-off-by: Alice Ryhl --- drivers/android/binder/process.rs | 67 ++++++++++++++++--------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs index 240386e981cd..23d0f2af426c 100644 --- a/drivers/android/binder/process.rs +++ b/drivers/android/binder/process.rs @@ -931,7 +931,7 @@ impl Process { refs.by_node.remove(&id); } } else { - pr_warn!("{}: no such ref {handle}\n", kernel::current!().pid()); + pr_warn!("{}: no such ref {handle}\n", self.pid_in_current_ns()); } Ok(()) } @@ -1310,37 +1310,9 @@ impl Process { work.into_arc().cancel(); } - // Take all threads and release them. - let threads = take(&mut self.inner.lock().threads); - for thread in threads.values() { - thread.release(); - } - drop(threads); - - // Free any resources kept alive by allocated buffers. - let omapping = self.inner.lock().mapping.take(); - if let Some(mut mapping) = omapping { - let address = mapping.address; - mapping - .alloc - .take_for_each(|offset, size, debug_id, odata| { - let ptr = offset + address; - let mut alloc = - Allocation::new(self.clone(), debug_id, offset, size, ptr, false); - if let Some(data) = odata { - alloc.set_info(data); - } - drop(alloc) - }); - } - // Drop all references. We do this dance with `swap` to avoid destroying the references // while holding the lock. - let mut refs = self.node_refs.lock(); - let mut node_refs = take(&mut refs.by_handle); - let freeze_listeners = take(&mut refs.freeze_listeners); - drop(refs); - for info in node_refs.values_mut() { + for info in self.node_refs.lock().by_handle.values_mut() { // SAFETY: We are removing the `NodeRefInfo` from the right node. unsafe { info.node_ref2().node.remove_node_info(&info) }; @@ -1352,14 +1324,24 @@ impl Process { }; death.set_cleared(false); } - drop(node_refs); + let freeze_listeners = take(&mut self.node_refs.lock().freeze_listeners); for listener in freeze_listeners.values() { listener.on_process_exit(&self); } drop(freeze_listeners); + // Do similar dance for the state lock. + let mut inner = self.inner.lock(); + let threads = take(&mut inner.threads); + let nodes = take(&mut inner.nodes); + drop(inner); + + // Release all threads. + for thread in threads.values() { + thread.release(); + } + // Deliver death notifications. - let nodes = take(&mut self.inner.lock().nodes); for node in nodes.values() { loop { let death = { @@ -1373,6 +1355,27 @@ impl Process { death.set_dead(); } } + + // Free any resources kept alive by allocated buffers. + let omapping = self.inner.lock().mapping.take(); + if let Some(mut mapping) = omapping { + let address = mapping.address; + mapping + .alloc + .take_for_each(|offset, size, debug_id, odata| { + let ptr = offset + address; + pr_warn!( + "{}: removing orphan mapping {offset}:{size}\n", + self.pid_in_current_ns() + ); + let mut alloc = + Allocation::new(self.clone(), debug_id, offset, size, ptr, false); + if let Some(data) = odata { + alloc.set_info(data); + } + drop(alloc) + }); + } } pub(crate) fn drop_outstanding_txn(&self) {