From 4d21f4cfc2cfd3166164218f665f89b9ec8a69a5 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Tue, 11 Sep 2018 19:11:05 +0300 Subject: [PATCH] init_ws_for_output: use workspace_move_to_output MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This fixes a crash produced with the following config: # i3 config file (v4) workspace 1 output $screen1 workspace 2 output $screen2 exec --no-startup-id "i3-msg workspace 1, open && i3-msg workspace 2 && xrandr --output $screen2 --off && xrandr --output $screen1 --auto --output $screen2 --auto --right-of $screen1 " Which results in: ERROR: AddressSanitizer: heap-use-after-free on address … READ of size 8 at 0x614000001f48 thread T0 #0 0x5563df6e73a8 in init_ws_for_output i3/src/randr.c:468 #1 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940 #2 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450 … is located 264 bytes inside of 448-byte region … freed by thread T0 here: #1 0x5563df634b0a in con_free i3/src/con.c:96 #2 0x5563df7151e6 in tree_close_internal i3/src/tree.c:344 #3 0x5563df7280fe in workspace_show i3/src/workspace.c:499 #4 0x5563df6e7315 in init_ws_for_output i3/src/randr.c:457 #5 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940 #6 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450 Which is similar to #3228, #3248. --- src/randr.c | 48 +++++++----------------------------------------- src/workspace.c | 10 +++++++++- 2 files changed, 16 insertions(+), 42 deletions(-) diff --git a/src/randr.c b/src/randr.c index b1a9e8cb..0c1f9704 100644 --- a/src/randr.c +++ b/src/randr.c @@ -442,47 +442,13 @@ void init_ws_for_output(Output *output, Con *content) { continue; } - /* if so, move it over */ - LOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n", - workspace->name, workspace_out->name, output_primary_name(output)); - - /* if the workspace is currently visible on that output, we need to - * switch to a different workspace - otherwise the output would end up - * with no active workspace */ - bool visible = workspace_is_visible(workspace); - Con *previous = NULL; - if (visible && (previous = TAILQ_NEXT(workspace, focused))) { - LOG("Switching to previously used workspace \"%s\" on output \"%s\"\n", - previous->name, workspace_out->name); - workspace_show(previous); - } - - /* Render the output on which the workspace was to get correct Rects. - * Then, we need to work with the "content" container, since we cannot - * be sure that the workspace itself was rendered at all (in case it’s - * invisible, it won’t be rendered). */ - render_con(workspace_out, false); - Con *ws_out_content = output_get_content(workspace_out); - - Con *floating_con; - TAILQ_FOREACH(floating_con, &(workspace->floating_head), floating_windows) - /* NB: We use output->con here because content is not yet rendered, - * so it has a rect of {0, 0, 0, 0}. */ - floating_fix_coordinates(floating_con, &(ws_out_content->rect), &(output->con->rect)); - - con_detach(workspace); - con_attach(workspace, content, false); - - /* In case the workspace we just moved was visible but there was no - * other workspace to switch to, we need to initialize the source - * output as well */ - if (visible && previous == NULL) { - LOG("There is no workspace left on \"%s\", re-initializing\n", - workspace_out->name); - init_ws_for_output(get_output_by_name(workspace_out->name, true), - output_get_content(workspace_out)); - DLOG("Done re-initializing, continuing with \"%s\"\n", output_primary_name(output)); - } + DLOG("Moving workspace \"%s\" from output \"%s\" to \"%s\" due to assignment\n", + workspace->name, workspace_out->name, output_primary_name(output)); + /* Need to copy output's rect since content is not yet rendered. We + * can't call render_con here because render_output only proceeds if a + * workspace exists. */ + content->rect = output->con->rect; + workspace_move_to_output(workspace, output); } /* if a workspace exists, we are done now */ diff --git a/src/workspace.c b/src/workspace.c index e92aaa5f..2a9f88db 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -966,7 +966,11 @@ bool workspace_move_to_output(Con *ws, Output *output) { LOG("got output %p with content %p\n", output, content); Con *previously_visible_ws = TAILQ_FIRST(&(content->focus_head)); - LOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name); + if (previously_visible_ws) { + DLOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name); + } else { + DLOG("No previously visible workspace on output.\n"); + } bool workspace_was_visible = workspace_is_visible(ws); if (con_num_children(ws->parent) == 1) { @@ -1024,6 +1028,10 @@ bool workspace_move_to_output(Con *ws, Output *output) { workspace_show(ws); } + if (!previously_visible_ws) { + return true; + } + /* NB: We cannot simply work with previously_visible_ws since it might * have been cleaned up by workspace_show() already, depending on the * focus order/number of other workspaces on the output. -- 2.39.5