Repository navigation
BackgroundManager: Guard against actors that do not exist yet - #2960
Open
eltictacdicta wants to merge 1 commit into
Open
eltictacdicta wants to merge 1 commit into
eltictacdicta wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Gala.BackgroundManagertouches 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 actorset_size()dereferencesbackground_actorbefore the first background has finished loading:At that point
background_actoris stillnull: it is only assigned byswap_background_actor()once the asynchronousBackground.loadedhandler fires.WorkspaceClone.update_targets()callsset_size()while the multitasking view is being built, that is, during startup, so every session logs:Confirmed with gdb against the packaged 8.6.2 build (
G_DEBUG=fatal-criticals,gala --nested):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 sharedBackgrounddelegate, but that handler is not tied to the actor it was created for: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 leavesnew_background_actor == nullfor the handler that actually belongs to the new actor, which then trips the precondition: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
mainwith identical flags (-Dbuildtype=debugoptimized, mutter 46.2) and exercised withgala --nested --waylandfor 15 seconds each. Only the patch differs:clutter_actor_get_width: assertion 'CLUTTER_IS_ACTOR (self)' failedswap_background_actor: assertion 'new_background_actor != null' failedScope and risk
set_size()keeps working exactly as before.BackgroundManager.valais byte-identical betweenmainandnoble, and this change was built and tested against mutter 46, so it backports to the 8.6 branch if you want it there.BackgroundContainerdestructor,x11_set_window_pass_through,thaw_remove). Those are separate defects.