virtmgr: Clear callbacks when unused
This is the defensive code to prevent cyclic references between
callback and VmInstance, which can happen when callback
implementation holds strong reference of VmInstance.
The cyclic references is OK while VM is alive, but it prevents
VmInstance from being dropped even after VM is died.
Test: T/H
Change-Id: I02411a19e26c4ea6156476c5d1d48508131e16a8
diff --git a/android/virtmgr/src/aidl.rs b/android/virtmgr/src/aidl.rs
index 2d31b87..ffcc812 100644
--- a/android/virtmgr/src/aidl.rs
+++ b/android/virtmgr/src/aidl.rs
@@ -1578,9 +1578,17 @@
) -> binder::Result<()> {
// Don't check permission. The owner of the VM might have passed this binder object to
// others.
- //
- // TODO: Should this give an error if the VM is already dead?
- self.instance.callbacks.add(callback.clone());
+
+ // Only register callback if it may be notified.
+ // This also ensures no cyclic reference between callback and VmInstance after VM is died.
+ let vm_state = self.instance.vm_state.lock().unwrap();
+ if matches!(*vm_state, VmState::Dead) {
+ warn!("Ignoring registerCallback() after VM is died");
+ } else {
+ self.instance.callbacks.add(callback.clone());
+ }
+ drop(vm_state);
+
Ok(())
}
@@ -1746,12 +1754,17 @@
/// Call all registered callbacks to say that the VM has died.
pub fn callback_on_died(&self, cid: Cid, reason: DeathReason) {
- let callbacks = &*self.0.lock().unwrap();
- for callback in callbacks {
+ let mut callbacks = self.0.lock().unwrap();
+ for callback in &*callbacks {
if let Err(e) = callback.onDied(cid as i32, reason) {
error!("Error notifying exit of VM CID {}: {:?}", cid, e);
}
}
+
+ // Nothing to notify afterward because VM cannot be restarted.
+ // Explicitly clear callbacks to prevent potential cyclic references
+ // between callback and VmInstance.
+ (*callbacks).clear();
}
/// Add a new callback to the set.