Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mutter 3.3x crashes #635

Merged
merged 8 commits into from
Nov 13, 2019
Merged

Conversation

Tireg
Copy link
Contributor

@Tireg Tireg commented Nov 8, 2019

This PR fixes issues against running upstream gala with mutter 3.3x.
This should fix #634

Every single patch is detailed in this post: #634 (comment)
I reported every step as a commit comment in order to understand it more easily.

@Tireg Tireg changed the title Wip/fix mutter 3.3x crashes Fix mutter 3.3x crashes Nov 8, 2019
Copy link
Member

@tintou tintou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By creating an unowned List you're creating a leak here

src/Widgets/MultitaskingView.vala Outdated Show resolved Hide resolved
src/WorkspaceManager.vala Outdated Show resolved Hide resolved
src/WorkspaceManager.vala Outdated Show resolved Hide resolved
@Tireg
Copy link
Contributor Author

Tireg commented Nov 13, 2019

By creating an unowned List you're creating a leak here

For each one of the suggestions, we fill the list using manager.get_workspace_by_index (i). I don't know Vala but isn't there something like List<unowned Workspace> (instead of List<Workspace>) that I should use in order to avoid destroying the Workspaces objects when the List is destroyed ?

Just to be sure there is no mistake.

Edit:

Ok I just tested it, it seems to work without unowned so I added the changes.
Not quite sure to understand why unowned is not needed however.

I just noticed I should have accepted the changes directly from github instead of pushing a new commit however.

@Tireg Tireg requested a review from tintou November 13, 2019 20:46
@tintou
Copy link
Member

tintou commented Nov 13, 2019

@Tireg a new commit is fine too, no worries :)
The difference here:
List<unowned Workspace> means that we have to free the List but not the elements
List<Workspace> means that we have to free the list and unref the elements
Defined as List<Workspace>, Vala will take care to increase the reference count before adding an element to the list, when List<unowned Workspace> would not ref the workspace, so List<Workspace> is safer for us in this case.

@tintou tintou merged commit 70cfe7e into elementary:master Nov 13, 2019
@Tireg Tireg deleted the wip/fix-mutter-3.3x-crashes branch November 13, 2019 21:13
@decathorpe
Copy link

I'm looking forward to testing this on fedora :)

Just one question / nit-pick: The name for the gsettings override file seems rather odd:

20_elementary.pantheon.wm.gschema.override

Can we rename it to something like 20_io.elementary.gala.gschema.override instead?

@tintou
Copy link
Member

tintou commented Nov 14, 2019

@decathorpe Yeah, would make sense too :)

@decathorpe
Copy link

Hm. I'm still getting a reproducible crash on startup on fedora 31 :( I'll try to file a new bug report for it, once I'm able to provide a useful traceback :(

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.

Gala crash on startup with mutter 3.3x
3 participants