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

i3 crashes when renaming an empty workspace such that it is moved to an output with a focused nonempty workspace #3248

Closed
ograff opened this Issue Apr 17, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@ograff
Contributor

ograff commented Apr 17, 2018

Output of i3 --moreversion 2>&- || i3 --version:

Binary i3 version:  4.15-108-g67a60a94 (2018-04-16, branch "next")

URL to a logfile as per https://i3wm.org/docs/debugging.html:

https://logs.i3wm.org/logs/5737979670691840.bz2

What I did:

Used the following i3 config:

# i3 config file (v4)

workspace 5 output fake-1
fake-outputs 800x500+0+0P,800x500+800+0,800x500+800+500,800x500+0+500

Ran

i3-msg rename workspace 5 to 6, workspace 6, exec gvim

Waited for gvim to appear
ran

i3-msg rename workspace 1 to 5

What I saw:

i3 crashed with the following traces:

==7355==ERROR: AddressSanitizer: heap-use-after-free on address 0x61400000149c at pc 0x55e68a1a5a7d bp 0x7ffcf4e29110 sp 0x7ffcf4e29100
READ of size 4 at 0x61400000149c thread T0
    #0 0x55e68a1a5a7c in dump_node ../../i3/src/ipc.c:252
    #1 0x55e68a1addad in ipc_marshal_workspace_event ../../i3/src/ipc.c:1414
    #2 0x55e68a1adeee in ipc_send_workspace_event ../../i3/src/ipc.c:1435
    #3 0x55e68a1652cc in cmd_rename_workspace ../../i3/src/commands.c:2026
    #4 0x55e68a166c48 in GENERATED_call parser/GENERATED_command_call.h:22
    #5 0x55e68a16976d in next_state ../../i3/src/commands_parser.c:185
    #6 0x55e68a16ab94 in parse_command ../../i3/src/commands_parser.c:346
    #7 0x55e68a1a4f8a in handle_run_command ../../i3/src/ipc.c:124
    #8 0x55e68a1ad1d4 in ipc_receive_message ../../i3/src/ipc.c:1304
    #9 0x7f2d0154ad72 in ev_invoke_pending (/usr/lib/x86_64-linux-gnu/libev.so.4+0x3d72)
    #10 0x7f2d0154e3dd in ev_run (/usr/lib/x86_64-linux-gnu/libev.so.4+0x73dd)
    #11 0x55e68a1b4c2e in ev_loop /usr/include/ev.h:835
    #12 0x55e68a1bd98f in main ../../i3/src/main.c:965
    #13 0x7f2d00c131c0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x211c0)
    #14 0x55e68a13dca9 in _start (/home/ograff/i3/build/i3+0x4dca9)

0x61400000149c is located 92 bytes inside of 448-byte region [0x614000001440,0x614000001600)
freed by thread T0 here:
    #0 0x7f2d0444b7b8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8)
    #1 0x55e68a16c870 in con_free ../../i3/src/con.c:96
    #2 0x55e68a1e7d12 in tree_close_internal ../../i3/src/tree.c:340
    #3 0x55e68a1f29be in workspace_show ../../i3/src/workspace.c:469
    #4 0x55e68a1650d3 in cmd_rename_workspace ../../i3/src/commands.c:2015
    #5 0x55e68a166c48 in GENERATED_call parser/GENERATED_command_call.h:22
    #6 0x55e68a16976d in next_state ../../i3/src/commands_parser.c:185
    #7 0x55e68a16ab94 in parse_command ../../i3/src/commands_parser.c:346
    #8 0x55e68a1a4f8a in handle_run_command ../../i3/src/ipc.c:124
    #9 0x55e68a1ad1d4 in ipc_receive_message ../../i3/src/ipc.c:1304
    #10 0x7f2d0154ad72 in ev_invoke_pending (/usr/lib/x86_64-linux-gnu/libev.so.4+0x3d72)

previously allocated by thread T0 here:
    #0 0x7f2d0444bd38 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded38)
    #1 0x55e68a210045 in scalloc ../../i3/libi3/safewrappers.c:31
    #2 0x55e68a16bb79 in con_new_skeleton ../../i3/src/con.c:40
    #3 0x55e68a16c1c3 in con_new ../../i3/src/con.c:71
    #4 0x55e68a1f0cad in create_workspace_on_output ../../i3/src/workspace.c:202
    #5 0x55e68a1ce338 in init_ws_for_output ../../i3/src/randr.c:516
    #6 0x55e68a190b1b in fake_outputs_init ../../i3/src/fake_outputs.c:77
    #7 0x55e68a1bb641 in main ../../i3/src/main.c:714
    #8 0x7f2d00c131c0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x211c0)

What I expected instead:
I'm not sure of the correct behavior here. As documented on #3228, there is not a clear correct behavior.

It appears the issue here is the workspace being renamed is cleaned up because it is empty and not in focus. After it goes out of focus (and is cleaned up) we attempt to access some of its information in order to populate an IPC message.

Some possible behaviors put forth on #3228:

  • Focus the workspace being renamed in this particular scenario
  • Always focus the workspace being renamed
  • Reject the rename in this scenario
  • Focus the previous workspace after all interactions with the workspace being renamed are complete.
  • Somehow flag renamed workspaces as "Do not delete until the user looks at this next"

Another idea:

  • Save the data to be sent over ipc before the workspace being renamed could be deleted.

@i3bot i3bot added the 4.15 label Apr 17, 2018

@ograff ograff changed the title from i3 crashes when renaming an empty workspace such that it is moved to an output with a nonempty workspace to i3 crashes when renaming an empty workspace such that it is moved to an output with a focused nonempty workspace Apr 17, 2018

@orestisf1993

This comment has been minimized.

Member

orestisf1993 commented Apr 17, 2018

This is after #3245 is applied. Else we trigger #3228 again, right?

@ograff

This comment has been minimized.

Contributor

ograff commented Apr 17, 2018

Nope, this issue can be triggered without #3245 being applied. #3228 describes an issue where the currently focused workspace has no windows. This describes the situation where the currently focused workspace has a window, but the workspace being renamed does not.

In situations where both workspaces have windows, no crash occurs because neither is cleaned up.

orestisf1993 added a commit to orestisf1993/i3 that referenced this issue Sep 6, 2018

cmd_rename_workspace: correct order of events
1. Rename happens
2. Workspace is moved because of assignments
3. Workspace closes because it is empty (i3#3248)

Fixes i3#3248.

orestisf1993 added a commit to orestisf1993/i3 that referenced this issue Sep 11, 2018

init_ws_for_output: use workspace_move_to_output
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:
==12297==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
    i3#1 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    i3#2 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

0x614000001f48 is located 264 bytes inside of 448-byte region [0x614000001e40,0x614000002000)
freed by thread T0 here:
    #0 0x7f4fd7d89c19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:66
    i3#1 0x5563df634b0a in con_free i3/src/con.c:96
    i3#2 0x5563df7151e6 in tree_close_internal i3/src/tree.c:344
    i3#3 0x5563df7280fe in workspace_show i3/src/workspace.c:499
    i3#4 0x5563df6e7315 in init_ws_for_output i3/src/randr.c:457
    i3#5 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    i3#6 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

Which is similar to i3#3228, i3#3248.

orestisf1993 added a commit to orestisf1993/i3 that referenced this issue Sep 11, 2018

init_ws_for_output: use workspace_move_to_output
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
    i3#1 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    i3#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:
    i3#1 0x5563df634b0a in con_free i3/src/con.c:96
    i3#2 0x5563df7151e6 in tree_close_internal i3/src/tree.c:344
    i3#3 0x5563df7280fe in workspace_show i3/src/workspace.c:499
    i3#4 0x5563df6e7315 in init_ws_for_output i3/src/randr.c:457
    i3#5 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    i3#6 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

Which is similar to i3#3228, i3#3248.

orestisf1993 added a commit to orestisf1993/i3 that referenced this issue Sep 12, 2018

init_ws_for_output: use workspace_move_to_output
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
    i3#1 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    i3#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:
    i3#1 0x5563df634b0a in con_free i3/src/con.c:96
    i3#2 0x5563df7151e6 in tree_close_internal i3/src/tree.c:344
    i3#3 0x5563df7280fe in workspace_show i3/src/workspace.c:499
    i3#4 0x5563df6e7315 in init_ws_for_output i3/src/randr.c:457
    i3#5 0x5563df6ef3b4 in randr_query_outputs i3/src/randr.c:940
    i3#6 0x5563df68dbe1 in handle_screen_change i3/src/handlers.c:450

Which is similar to i3#3228, i3#3248.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment