Skip to content

clutter/actor: First clear all stage views before emitting they changed

Jonas Ådahl requested to merge jadahl/mutter:wip/timeline-clear-tree into main

If one would end up with an actor attached to mapped actor, where the attached actor doesn't itself have an up to date stage view list while listening on the stage for updating, when clearing the stage views of the list, anything that would query the stage views list at this time would end up accessing freed memory.

This could happen if

  1. An actor was added to a newly created container actor attached to the stage
  2. The actor got a timeline attached to it
  3. The actor was moved to a container that already was mapped
  4. A hotplug happened

After (1) both the container and actor would not have any stage views. After (2) the timeline would listen on the stage for stage views updates. After (3) the actor would still listen on the stage for stage views updates. When (4) happened, the actor would be signalled when the stage got its stage view cleared, at which point it would traverse up its actor's tree finding an appropriate stage view to base its animation on. The problem here would be that it'd query the already mapped container and its yet-to-be-cleared stage view list, resulting in use-after free, resulting in for example the following backtrace:

  1. g_type_check_instance_cast ()
  2. CLUTTER_STAGE_VIEW ()
  3. clutter_actor_pick_frame_clock ()
  4. clutter_actor_pick_frame_clock ()
  5. update_frame_clock ()
  6. on_frame_clock_actor_stage_views_changed ()
  7. g_closure_invoke ()
  8. signal_emit_unlocked_R ()
  9. g_signal_emit_valist ()
  10. g_signal_emit ()
  11. clear_stage_views_cb ()
  12. _clutter_actor_traverse_depth ()
  13. _clutter_actor_traverse ()
  14. clutter_actor_clear_stage_views_recursive ()
  15. clutter_stage_clear_stage_views () ...

Avoid this issue by making sure that we don't emit 'stage-views-changed' signals while the actor tree is in an invalid state. While we now end up traversing tree twice, it doesn't change the Big-O notation. It has not been measured whether this has any noticible performance impact.

Closes: #1950 (closed)

Merge request reports