Skip to content

Commit

Permalink
Fix Gala.WorkspaceManager.cleanup method which causes SIGABRT by tryi…
Browse files Browse the repository at this point in the history 
…ng to get non-existing workspaces
  • Loading branch information
Julian committed Nov 8, 2019
1 parent fb2082e commit a2ad783
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/WorkspaceManager.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,12 @@ namespace Gala
unowned Meta.Display display = wm.get_display ();
unowned Meta.WorkspaceManager manager = display.get_workspace_manager ();
var last_index = manager.get_n_workspaces () - 1;
unowned List<Meta.Workspace> workspaces = null;
for (int i = 0; i < manager.get_n_workspaces (); i++) {
unowned Meta.Workspace workspace = manager.get_workspace_by_index (i);
workspaces.append (manager.get_workspace_by_index (i));
}

foreach (var workspace in workspaces) {
if (Utils.get_n_windows (workspace) < 1
&& workspace.index () != last_index) {
remove_workspace (workspace);
Expand Down

1 comment on commit a2ad783

@Tireg
Copy link
Contributor

@Tireg Tireg commented on a2ad783 Nov 8, 2019

Choose a reason for hiding this comment

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

Issue:

Gala still crashes on startup.

Description of the fix:

Ok so fine, workspaces are removed properly but this still crashes. Let's go back to the Gala.WorkspaceManager.cleanup () function:

unowned Meta.Display display = wm.get_display ();
unowned Meta.WorkspaceManager manager = display.get_workspace_manager ();
var last_index = manager.get_n_workspaces () - 1;
for (int i = 0; i < manager.get_n_workspaces (); i++) {
	unowned Meta.Workspace workspace = manager.get_workspace_by_index (i);
	if (Utils.get_n_windows (workspace) < 1
		&& workspace.index () != last_index) {
			remove_workspace (workspace);
	}
}

This seems to differ from the old behaviour:

var screen = wm.get_screen ();
var last_index = screen.get_n_workspaces () - 1;
unowned GLib.List<Meta.Workspace> workspaces = screen.get_workspaces ();

foreach (var workspace in workspaces) {
	if (Utils.get_n_windows (workspace) < 1
		&& workspace.index () != last_index) {
			remove_workspace (workspace);
	}
}

We are now looping with indexes directly in order to delete workspaces. This is problematic: the loop delete workspaces, and so the workspace indexes may changes at each loop count. We need to replicate the old behaviour:

unowned Meta.Display display = wm.get_display ();
unowned Meta.WorkspaceManager manager = display.get_workspace_manager ();
var last_index = manager.get_n_workspaces () - 1;
unowned List<Meta.Workspace> existing_workspaces = null;
for (int i = 0; i < manager.get_n_workspaces (); i++) {
	existing_workspaces.append (manager.get_workspace_by_index (i));
}

foreach (var workspace in workspaces) {
	if (Utils.get_n_windows (workspace) < 1
		&& workspace.index () != last_index) {
			remove_workspace (workspace);
	}
}

Please sign in to comment.