Skip to content

BackgroundManager: Guard against actors that do not exist yet - #2960

Open
eltictacdicta wants to merge 1 commit into
elementary:mainfrom
eltictacdicta:fix/background-actor-guards
Open

eltictacdicta wants to merge 1 commit into
elementary:mainfrom
eltictacdicta:fix/background-actor-guards

Conversation

@eltictacdicta

Copy link
Copy Markdown

Summary

Gala.BackgroundManager touches background actors that do not exist yet. On every session start the journal is flooded with Clutter criticals, and the background is left unallocated for that pass. There are two independent paths; both are fixed here in 13 lines, one file.

Defect 1 — set_size() reads a null actor

set_size() dereferences background_actor before the first background has finished loading:

public new void set_size (float width, float height) {
    if (width != background_actor.width || height != background_actor.height) {

At that point background_actor is still null: it is only assigned by swap_background_actor() once the asynchronous Background.loaded handler fires. WorkspaceClone.update_targets() calls set_size() while the multitasking view is being built, that is, during startup, so every session logs:

Clutter-CRITICAL **: clutter_actor_get_width: assertion 'CLUTTER_IS_ACTOR (self)' failed

Confirmed with gdb against the packaged 8.6.2 build (G_DEBUG=fatal-criticals, gala --nested):

#2  clutter_actor_get_width            libmutter-clutter-14.so.0
#3  gala_background_manager_set_size   gala
#4  ...
#5  ...
#18 gala_layout_manager_init_ui        gala

Skipping the call while the actor does not exist loses nothing: create_background_actor() already applies the same monitor geometry (display.get_monitor_geometry (monitor_index)) to the actor as soon as it is created.

Defect 2 — the async handler can swap a stale actor

update_background_actor() connects a one-shot handler to the shared Background delegate, but that handler is not tied to the actor it was created for:

handler = background.loaded.connect (() => {
    ...
    swap_background_actor (animate);
});

If update_background_actor() runs again while a load is in flight (monitor reconfiguration, set_size(), wallpaper change), the field is replaced and the stale handler later swaps the new actor in prematurely. That leaves new_background_actor == null for the handler that actually belongs to the new actor, which then trips the precondition:

CRITICAL **: gala_background_manager_swap_background_actor: assertion 'new_background_actor != null' failed

The fix compares the actor identity before swapping, so an out-of-date load result is dropped instead of corrupting the swap.

Verification

Both trees were built from main with identical flags (-Dbuildtype=debugoptimized, mutter 46.2) and exercised with gala --nested --wayland for 15 seconds each. Only the patch differs:

Critical unpatched patched
clutter_actor_get_width: assertion 'CLUTTER_IS_ACTOR (self)' failed 1 0
swap_background_actor: assertion 'new_background_actor != null' failed 4 0

Scope and risk

  • Behaviour is unchanged whenever the actor exists: set_size() keeps working exactly as before.
  • No new state, no new signal, no refactor: only two early returns.
  • BackgroundManager.vala is byte-identical between main and noble, and this change was built and tested against mutter 46, so it backports to the 8.6 branch if you want it there.
  • This does not address the SIGSEGV/SIGABRT signatures reported in Gala crashes repeatedly on X11 session when external HDMI monitor is connected #2843 (BackgroundContainer destructor, x11_set_window_pass_through, thaw_remove). Those are separate defects.

set_size() reads background_actor while the first background is still
loading, when the field is null. WorkspaceClone.update_targets() calls
it during multitasking view construction, so Clutter logs
"clutter_actor_get_width: assertion 'CLUTTER_IS_ACTOR (self)' failed"
on every startup. create_background_actor() already applies the same
monitor geometry once the actor exists, so skipping the call is safe.

The async loaded handler is bound to the shared Background delegate
instead of the actor it was created for. If update_background_actor()
runs again while a load is in flight, the stale handler swaps the wrong
actor in and leaves new_background_actor null, tripping the
swap_background_actor() precondition. Check the actor identity before
swapping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant