Fix unsound observer initialization

crates/bevy_ecs/src/observer/distributed_storage.rs

@@ -207,7 +207,7 @@ use crate::prelude::ReflectComponent;
 pub struct Observer {
     hook_on_add: ComponentHook,
     pub(crate) error_handler: Option<ErrorHandler>,
-    pub(crate) system: Box<dyn AnyNamedSystem>,
+    pub(crate) system: Option<Box<dyn AnyNamedSystem>>,
     pub(crate) descriptor: ObserverDescriptor,
     pub(crate) last_trigger_id: u32,
     pub(crate) despawned_watched_entities: u32,
@@ -224,7 +224,7 @@ impl Observer {
     pub fn new<E: EventPattern, M, I: IntoObserverSystem<E, M>>(system: I) -> Self {
         let system = Box::new(IntoObserverSystem::into_system(system));
         Self {
-            system,
+            system: Some(system),
             descriptor: Default::default(),
             hook_on_add: hook_on_add::<E, I::System>,
             error_handler: None,
@@ -238,7 +238,7 @@ impl Observer {
     /// Creates a new [`Observer`] with custom runner, this is mostly used for dynamic event observers
     pub fn with_dynamic_runner(runner: ObserverRunner) -> Self {
         Self {
-            system: Box::new(IntoSystem::into_system(|| {})),
+            system: Some(Box::new(IntoSystem::into_system(|| {}))),
             descriptor: Default::default(),
             hook_on_add: |mut world, hook_context| {
                 let default_error_handler = world.fallback_error_handler();
@@ -351,7 +351,10 @@ impl Observer {
 
     /// Returns the name of the [`Observer`]'s system .
     pub fn system_name(&self) -> DebugName {
-        self.system.system_name()
+        self.system.as_deref().map_or(
+            DebugName::borrowed("<system is initializing>"),
+            AnyNamedSystem::system_name,
+        )
     }
 }
 
@@ -453,43 +456,38 @@ fn hook_on_add<E: EventPattern, S: ObserverSystem<E>>(
         let event_key = world.register_event_key::<E::Event>();
         let components = E::Components::component_ids(&mut world.components_registrator());
 
-        let system_ptr: *mut dyn ObserverSystem<E> = {
-            let Some(mut observer) = world.get_mut::<Observer>(entity) else {
-                return;
-            };
-            observer.descriptor.event_keys.push(event_key);
-            observer.descriptor.components.extend(components);
-
-            let system: &mut dyn Any = observer.system.as_mut();
-            core::ptr::from_mut(system.downcast_mut::<S>().unwrap())
+        let Some(mut observer) = world.get_mut::<Observer>(entity) else {
+            return;
         };
+        observer.descriptor.event_keys.push(event_key);
+        observer.descriptor.components.extend(components);
 
-        // SAFETY: World reference is exclusive and initialize does not touch system, so references do not alias
-        let access = unsafe { (*system_ptr).initialize(world) };
+        let mut boxed_system = core::mem::take(&mut observer.system).unwrap();
+        let mut conditions = core::mem::take(&mut observer.conditions);
+
+        let system: &mut dyn Any = boxed_system.as_mut();
+        let system = system.downcast_mut::<S>().unwrap();
+        let access = system.initialize(world);
         assert!(
             !access.is_exclusive(),
             concat!(
                 "Exclusive system `{}` may not be used as observer.\n",
                 "Instead of `&mut World`, use either `DeferredWorld` if you do not need structural changes, or `Commands` if you do."
             ),
-            // SAFETY: World reference is exclusive and initialize does not touch system, so references do not alias
-            unsafe { (*system_ptr).name() }
+            system.name(),
         );
 
-        let mut conditions = {
-            let Some(mut observer) = world.get_mut::<Observer>(entity) else {
-                return;
-            };
-            core::mem::take(&mut observer.conditions)
-        };
-
         for condition in &mut conditions {
             condition.initialize(world);
         }
 
-        if let Some(mut observer) = world.get_mut::<Observer>(entity) {
-            observer.conditions = conditions;
-        }
+        // If the observer was despawned during `initialize`, don't register it.
+        let Some(mut observer) = world.get_mut::<Observer>(entity) else {
+            return;
+        };
+
+        observer.system = Some(boxed_system);
+        observer.conditions = conditions;
 
         world.register_observer(entity);
     });

crates/bevy_ecs/src/observer/mod.rs

@@ -1925,4 +1925,69 @@ mod tests {
 
         assert!(!world.entity(target).contains::<ObservedBy>());
     }
+
+    #[test]
+    fn observer_system_despawns_observer() {
+        let mut world = World::new();
+        world.add_observer(DespawnObserversOnInit(0));
+
+        use crate::change_detection::{CheckChangeTicks, Tick};
+        use crate::system::{RunSystemError, SystemAccess, SystemStateFlags};
+        use crate::world::unsafe_world_cell::UnsafeWorldCell;
+        use bevy_utils::prelude::DebugName;
+
+        #[expect(unused, reason = "This will only trigger UB if it has nonzero size")]
+        struct DespawnObserversOnInit(usize);
+        impl System for DespawnObserversOnInit {
+            type In = On<'static, 'static, Add<()>>;
+
+            type Out = ();
+
+            fn name(&self) -> DebugName {
+                DebugName::type_name::<DespawnObserversOnInit>()
+            }
+
+            fn flags(&self) -> SystemStateFlags {
+                SystemStateFlags::empty()
+            }
+
+            unsafe fn run_unsafe(
+                &mut self,
+                _input: SystemIn<'_, Self>,
+                _world: UnsafeWorldCell,
+            ) -> Result<Self::Out, RunSystemError> {
+                Ok(())
+            }
+
+            #[cfg(feature = "hotpatching")]
+            fn refresh_hotpatch(&mut self) {}
+
+            fn apply_deferred(&mut self, _world: &mut World) {}
+
+            fn queue_deferred(&mut self, _world: DeferredWorld) {
+                todo!()
+            }
+
+            fn initialize(&mut self, world: &mut World) -> SystemAccess {
+                let observers: Vec<_> = world
+                    .query_filtered::<Entity, With<Observer>>()
+                    .query(world)
+                    .iter()
+                    .collect();
+                for observer in observers {
+                    world.despawn(observer);
+                }
+
+                SystemAccess::None
+            }
+
+            fn check_change_tick(&mut self, _check: CheckChangeTicks) {}
+
+            fn get_last_run(&self) -> Tick {
+                unimplemented!()
+            }
+
+            fn set_last_run(&mut self, _last_run: Tick) {}
+        }
+    }
 }

crates/bevy_ecs/src/observer/runner.rs

@@ -88,7 +88,7 @@ pub(super) unsafe fn observer_system_runner<E: EventPattern, S: ObserverSystem<E
     // - observer was triggered so must have an `Observer` component.
     // - observer cannot be dropped or mutated until after the system pointer is already dropped.
     let system: *mut dyn ObserverSystem<E> = unsafe {
-        let system: &mut dyn Any = state.system.as_mut();
+        let system: &mut dyn Any = state.system.as_deref_mut().debug_checked_unwrap();
         let system = system.downcast_mut::<S>().debug_checked_unwrap();
         &mut *system
     };