* Have multiple outputs
* Launch swaylock
* Unplug an output (possibly has to be the last "connected" one)
* The swaylock surface on the remaining output would not respond to key
events
This was happening because when the output destroys, focus was not given
to the other swaylock surface.
This patch makes focus be transferred to another surface owned by the
same Wayland client, but only if input was inhibited by the surface
being destroyed, and only if it's in the overlay layer. I figure it's
best to be overly specific and relax the requirements later if needed.
This patch removes a check in seat_set_focus_surface which was
preventing focus from being passed from a layer surface to any other
surface. I don't know of a use case for this check, but it's possible
that this change could produce issues.
This replaces view.using_csd with a new border mode: B_CSD. This also
removes sway_xdg_shell{_v6}_view.deco_mode and
view->has_client_side_decorations as we can now get these from the
border.
You can use `border toggle` to cycle through the modes including CSD, or
use `border csd` to set it directly. The client must support the
xdg-decoration protocol, and the only client I know of that does is the
example in wlroots.
If the client switches from SSD to CSD without us expecting it (via the
server-decoration protocol), we stash the previous border type into
view.saved_border so we can restore it if the client returns to SSD. I
haven't found a way to test this though.
This adds a `con` argument to `execute_command` which allows you to
specify the container to execute the command on. In most cases it leaves
it as `NULL` which makes it use the focused node. We only set it when
executing `for_window` criteria such as when a view maps. This means we
don't send unnecessary IPC focus events, and fixes a crash when the
criteria command is `move scratchpad` (because we can't give focus to a
hidden scratchpad container).
Each of the shell map handlers now check to see if the view has a
workspace. It won't have a workspace if criteria has moved it to the
scratchpad.
When destroying an idle-inhibiting client, idle_inhibit_v1_check_active can get
called from transaction_progress_queue on a view with a null container.
view_is_visible does not handle a view in this state.
This does the following:
* Adds a baseline argument to get_text_size (the baseline is the
distance from the top of the texture to the baseline).
* Stores the baseline in the container when calculating the title
height.
* Takes the baseline into account when calculating the config's max font
height.
* When rendering, pads the textures according to the baseline so they
line up.
There was code that attempted to fill in the gap below the title texture
when the texture isn't tall enough, but this only worked when the output
was positioned at 0,0. The reason is that render_rect expects a box
passed in a hybrid layout-local/output-buffer-local system, and we were
passing purely output-buffer-local. I've added a comment documenting
this.
By the way, we can't use layout-local coordinates for the rectangle box
because in some cases we set the box based on a texture size. Texture
sizes are buffer-local, and we'd have to divide them to bring it back to
layout-local which means losing precision. We could use
output-buffer-local coordinates for the box, but this would require
translating the coordinates from layout-local to output-buffer-local in
many places during rendering.
This patch also vertically centers the text inside the title bar.
This fixes pinentry-gtk-2, but might make other views floating which
would otherwise be tiled. This patch is more of a trial which could end
up becoming a permanent fix.
When rendering, the workspace for the output needs to be retrieved from
the output's `current` state. output_get_active_workspace returns the
pending workspace, which crashes if the pending workspace is new and
hasn't completed a transaction yet.
Suppose the following:
* Transactions are already in progress - say transaction A.
* View A maps, which creates transaction B and appends it to the
transaction queue.
* View B maps, which creates transaction C and appends it to the queue.
* View A unmaps, which creates transaction D and appends it to the
queue.
* Transaction A completes, so transaction B attempts to save View A's
buffer, but this doesn't exist so it saves nothing.
* Rendering code attempts to render View A, but there is no saved buffer
nor live buffer that it can use.
Rather than implement an elaborate solution for a rare circumstance,
I've take the safe option of just not rendering anything for that view.
It means that if you reproduce the scenario above, you might get the
title and borders rendered but no surface.
This commit changes the meaning of sway_container so that it only refers
to layout containers and view containers. Workspaces, outputs and the
root are no longer known as containers. Instead, root, outputs,
workspaces and containers are all a type of node, and containers come in
two types: layout containers and view containers.
In addition to the above, this implements type safe variables. This
means we use specific types such as sway_output and sway_workspace
instead of generic containers or nodes. However, it's worth noting that
in a few places places (eg. seat focus and transactions) referring to
them in a generic way is unavoidable which is why we still use nodes in
some places.
If you want a TL;DR, look at node.h, as well as the struct definitions
for root, output, workspace and container. Note that sway_output now
contains a workspaces list, and workspaces now contain a tiling and
floating list, and containers now contain a pointer back to the
workspace.
There are now functions for seat_get_focused_workspace and
seat_get_focused_container. The latter will return NULL if a workspace
itself is focused. Most other seat functions like seat_get_focus and
seat_set_focus now accept and return nodes.
In the config->handler_context struct, current_container has been
replaced with three pointers: node, container and workspace. node is the
same as what current_container was, while workspace is the workspace
that the node resides on and container is the actual container, which
may be NULL if a workspace itself is focused.
The global root_container variable has been replaced with one simply
called root, which is a pointer to the sway_root instance.
The way outputs are created, enabled, disabled and destroyed has
changed. Previously we'd wrap the sway_output in a container when it is
enabled, but as we don't have containers any more it needs a different
approach. The output_create and output_destroy functions previously
created/destroyed the container, but now they create/destroy the
sway_output. There is a new function output_disable to disable an output
without destroying it.
Containers have a new view property. If this is populated then the
container is a view container, otherwise it's a layout container. Like
before, this property is immutable for the life of the container.
Containers have both a `sway_container *parent` and
`sway_workspace *workspace`. As we use specific types now, parent cannot
point to a workspace so it'll be NULL for containers which are direct
children of the workspace. The workspace property is set for all
containers, except those which are hidden in the scratchpad as they have
no workspace.
In some cases we need to refer to workspaces in a container-like way.
For example, workspaces have layout and children, but when using
specific types this makes it difficult. Likewise, it's difficult for a
container to get its parent's layout when the parent could be another
container or a workspace. To make it easier, some helper functions have
been created: container_parent_layout and container_get_siblings.
container_remove_child has been renamed to container_detach and
container_replace_child has been renamed to container_replace.
`container_handle_fullscreen_reparent(con, old_parent)` has had the
old_parent removed. We now unfullscreen the workspace when detaching the
container, so this function is simplified and only needs one argument
now.
container_notify_subtree_changed has been renamed to
container_update_representation. This is more descriptive of its
purpose. I also wanted to be able to call it with whatever container was
changed rather than the container's parent, which makes bubbling up to
the workspace easier.
There are now state structs per node thing. ie. sway_output_state,
sway_workspace_state and sway_container_state.
The focus, move and layout commands have been completely refactored to
work with the specific types. I considered making these a separate PR,
but I'd be backporting my changes only to replace them again, and it's
easier just to test everything at once.
Depends on https://github.com/swaywm/wlroots/pull/1222.
I don't know of a program that sets the state to modal without setting
the window type, but I know the modal property works because logging the
property shows it's true for the Firefox Open File dialog.
This moves the arrange_windows call into the arrange_layers function,
where we know the output actually needs to be arranged.
Additionally, we shouldn't set focus to the parent of an unknown
container type, because the parent may be an output and this causes a
crash because outputs can't have direct focus.
Fixes#2543
When collecting focus to save into the transaction state, the workspace
needs to look in the tiling list only.
As seat_get_focus_inactive_tiling returns any descendant, the list also
needs to be traversed back up to the direct child of the workspace.
Fixes#2532
When there's multiple transactions in the queue, sway can take a
shortcut by checking if they all operate on the same set of containers.
If they do, it can skip all but the last transaction. The way we tested
for transactions which used the same containers was to exclusive-or
their con IDs together, but this has proved not only to be ineffective
but also has the potential to make sway crash.
This patch replaces the exclusive-or with a loop and container
comparison.
* In layout command, arrange parent of parent - not sure why this is
needed but it is
* Remove gap adjustment when rendering
* Workspace should use outer gaps, not inner
* Add exceptions for tabbed and stacked containers
* Don't mess with gap state when splitting a container
This commit changes the arrange code in a way that will support type
safe arguments.
The arrange_output et al functions are now public, however I opted not
to use them directly yet. I've kept the generic arrange_windows there
for convenience until type safety is fully implemented. This means this
patch has much less risk of breaking things as it would otherwise.
To be type safe, arrange_children_of cannot exist in its previous form
because the thing passed to it could be either a workspace or a
container. So it's now renamed to arrange_children and accepts a list_t,
as well as the parent layout and parent's box.
There was some code which checked the grandparent's layout to see if it
was tabbed or stacked and adjusted the Y offset of the grandchild
accordingly. Accessing the grandparent layout isn't easy when using type
safe arguments, and it seemed odd to even need to do this. I determined
that this was needed because a child of a tabbed container would have a
swayc Y matching the top of the tab bar. I've changed this so a child of
a tabbed container will have a swayc Y matching the bottom of the tab
bar, which means we don't need to access the grandparent layout. Some
tweaks to the rendering and autoconfigure code have been made to
implement this, and the container_at code appears to work without
needing any changes.
arrange_children_of (now arrange_children) would check if the parent had
gaps and would copy them to the child, effectively making the
workspace's gaps recurse into all children. We can't do this any more
without passing has_gaps, gaps_inner and gaps_outer as arguments to
arrange_children, so I've changed the add_gaps function to retrieve it
from the workspace directly.
apply_tabbed_or_stacked_layout has been split into two functions, as it
had different logic depending on the layout.
Lastly, arrange.h had an unnecessary include of transaction.h. I've
removed it, which means I've had to add it to several other files.
When we have type safety we'll need to have functions for
workspace_add_tiling and so on. This means the existing container
functions will be just for containers, so they are being moved to
container.c. At this point layout.c doesn't contain much else, so I've
relocated everything and removed the file.
* container_swap and its static functions have been moved to the swap
command and made static.
* container_recursive_resize has been moved to the resize command and
made static.
* The following have been moved to container.c:
* container_handle_fullscreen_reparent
* container_insert_child
* container_add_sibling
* container_add_child
* container_remove_child
* container_replace_child
* container_split
* enum movement_direction and sway_dir_to_wlr have been moved to util.c.
Side note: Several commands included layout.h which then included
root.h. With layout.h gone, root.h has to be included by those commands.
This list includes disabled outputs.
When sway_container is demoted, we'll need to store the root's children
(ie. enabled outputs) in the sway_root. It makes sense to put these in a
list called `outputs`, so I'm renaming the existing list in advance.
This changes the destroy functions to the following:
* output_begin_destroy
* output_destroy
* workspace_begin_destroy
* workspace_destroy
* container_begin_destroy
* container_destroy
* view_begin_destroy
* view_destroy
The terminology was `destroy` and `free`, and it has been changed to
`begin_destroy` and `destroy` respectively.
When the last output is disconnected, its workspaces will now be stashed
in the root. Upon connection of a new output they will be restored.
There is a new function `workspace_consider_destroy` which decides
whether the given workspace should be destroyed or not (ie. empty and
not visible).
Calling container_begin_destroy will no longer automatically reap the
parents. In some places we want to reap the parents and in some we
don't, so this is left to the caller.
container_reap_empty_recursive and container_reap_empty have been
combined into one function and it will recurse up the tree.
Previously we used a reparent event to detect when a view changes
parent, then sent an output enter/leave to the surfaces if needed. This
worked for tiling views but not floating views, as floating views can
intersect another output without changing parent.
The solution implemented for floating views also applies cleanly to
tiling views, so the previous method has been completely replaced and
the reparent event has been removed.
This introduces a new function container_discover_outputs. This function
compares the container's `current` position to the outputs, sends enter
and leave events as needed, and keeps track of which outputs it's
intersecting in a new `container->outputs` list. If it has entered a new
output with a different scale then the title and marks textures will
also be recreated at the new scale.
The function is called when a transaction applies. This is convenient as
it means we don't have to call it from various places.
There is imperfect rendering when a floating view overlaps two outputs
with different scales. It renders correctly for the most recently
entered output, but there is only one title texture so it renders
incorrectly on the old output.
Fixes#2482
We were removing the saved buffer when one transaction applies, then
didn't have a new buffer to save when the next transaction ran. This
made the rendering code crash as it had no surface to use.
This commit makes it continue to hold the buffer if the view is
destroying and has more transactions. Additionally, a check is added
when saving the buffer to make sure there's no one already there.
Workspaces previously had a magical `workspace->floating` container,
which had a layout of L_FLOATING and whose children were actual floating
views. This allowed some conveniences, but was a hacky solution because
the container has to be exempt from focus, coordinate transactions with
the workspace, and omit emitting IPC events (which we didn't do).
This commit changes it to be a list directly in the sway_workspace. The
L_FLOATING layout is no longer used so this has been removed as well.
* Fixes incorrect check in the swap command (it checked if the
containers had the L_FLOATING layout, but this layout applied to the
magical container).
* Introduces workspace_add_floating
This makes all debug options stored in a single struct rather than in
various places, changes/fixes the behaviour of existing options, and
introduces some new options.
* Fixes damage issues with `-Drender-tree` texture (by removing scissor)
* Offsets the render tree overlay's `y` position for those who have
swaybar at the top
* Replaces `-Ddamage=rerender` with `-Dnodamage`
* Replaces `-Ddamage=highlight` with `-Dhighlight-damage`
* Replaces `-Dtxn-debug` with `-Dtxn-wait`
* Introduces `-Dnoatomic`
* Removes the `create_time` and `ms_arranging` figures from transactions
and the log message. Transactions are created after arranging and the
create time is of no significance.
* Fixes `-Dtxn-debug` (now `-Dtxn-wait`) not working.
This introduces the following `for_each` functions:
* root_for_each_workspace
* root_for_each_container
* output_for_each_workspace
* output_for_each_container
* workspace_for_each_container
And introduces the following `find` functions:
* root_find_output
* root_find_workspace
* root_find_container
* output_find_workspace
* output_find_container
* workspace_find_container
* container_find_child
And removes the following functions:
* container_descendants
* container_for_each_descendant
* container_find
This change is preparing the way for demoting sway_container. Eventually
these functions will accept and return sway_outputs, sway_workspaces and
sway_containers (meaning a C_CONTAINER or C_VIEW).
This change also makes it easy to handle abnormalities like the
workspace floating list, root's scratchpad list and (once implemented)
root's saved workspaces list for when there's no connected outputs.
This fixes a race condition flicker when unfloating a view which uses
client side decorations.
When the view is floated it has using_csd = true, so the decorations are
not drawn. When unfloating it it changes to false, but this change
wasn't part of transactions so it could potentially render the
decorations around the view while it's waiting for the transaction to
apply.
Commit 4b8e3a885b makes it so only one
transaction is committed (ie. configures sent) at a time. This commit
removes the now-unnecessary code which was used to support concurrent
committed transactions.
* Instead of containers storing a list of instructions which they've
been sent, it now stores a single instruction.
* Containers now have an ntxnrefs property. Previously we knew how many
references there were by the length of the instruction list.
* Instructions no longer need a ready property. It was used to avoid
marking an instruction ready twice when they were in a list, but this is
now avoided because there is only one instruction and we nullify the
container->instruction pointer when it's ready.
* When a transaction applies, we no longer need to consider releasing
and resaving the surface, as we know there are no other committed
transactions.
* transaction_notify_view_ready has been renamed to
view_notify_view_ready_by_serial to make it consistent with
transaction_notify_view_ready_by_size.
* Out-of-memory checks have been added when creating transactions and
instructions.
This fixes an issue where views might commit to a transaction ahead of
the first one, and applying the first transaction causes us to save a
buffer of the wrong size.
* The OP_RESIZE seat operation has been renamed to OP_RESIZE_FLOATING,
and OP_RESIZE_TILING has been introduced.
* Similar to the above, seat_begin_resize and handle_resize_motion have
been renamed and tiling variants introduced.
* resize.c's resize_tiled has to be used, so container_resize_tiled has
been introduced in resize.c to allow external code to call it.
This fixes two issues which were both introduced in #2396.
First issue:
The PR changes the location of the buffer save to transaction_apply, but
puts it inside the should_configure block. For unmapping (destroying)
views, should_configure returns false so it wasn't saving the buffer. If
a frame was rendered between the unmap and the transaction applying then
it would result in a crash.
Second issue:
If a destroying view is involved in two transactions, we must not
release the buffer between the transactions because there is no live
buffer to grab any more.
Fixes#2364.
Suppose a view is 600px wide, and we tell it to resize to 601px during a
resize operation. We create a transaction, save the 600px buffer and
send the configure. This buffer is saved into the associated
instruction, and is rendered while we wait for the view to commit a
601px buffer.
Before the view commits the 601px buffer, suppose we tell it to resize
to 602px. The new transaction will also save the buffer, but it's still
the 600px buffer because we haven't received a new one yet.
Then suppose the view commits its original 601px buffer. This completes
the first transaction, so we apply the 601px width to the container.
There's still the second (now only) transaction remaining, so we render
the saved buffer from that. But this is still the 600px buffer, and we
believe it's 601px. Whoops.
The problem here is we can't stack buffers like this. So this commit
removes the saved buffer from the instructions, places it in the view
instead, and re-saves the latest buffer every time the view completes a
transaction and still has further pending transactions.
As saved buffers are now specific to views rather than instructions, the
functions for saving and removing the saved buffer have been moved to
view.c.
The calls to save and restore the buffer have been relocated to more
appropriate functions too, favouring transaction_commit and
transaction_apply rather than transaction_add_container and
transaction_destroy.
Fixes the render and container_at order for popups.
Fixes#2210
For rendering:
* render_view_surfaces has been renamed to render_view_toplevels
* render_view_toplevels now uses output_surface_for_each_surface (which
is now public), as that function uses wlr_surface_for_each_surface which
doesn't descend into popups
* Views now have a for_each_popup iterator, which is used by the
renderer to render the focused view's popups
* When rendering a popup, toplevels (xdg subsurfaces) of that popup are
also rendered
For sending frame done, the logic has been updated to match the
rendering logic:
* send_frame_done_container no longer descends into popups
* for_each_popup is used to send frame done to the focused view's popups
and their child toplevels
For container_at:
* floating_container_at is now static, which means it had to be moved
higher in the file.
* container_at now considers popups for the focused view before checking
containers.
* tiling_container_at has been introduced, so that it doesn't call
container_at recursively (it would check popups recursively if it did)
Things worth noting:
* When a fullscreen view unmaps, the check to unset fullscreen on the
workspace has been moved out of view_unmap and into container_destroy,
because containers can be fullscreen too
* The calls to `container_reap_empty_recursive(workspace)` have been
removed from `container_set_floating`. That function reaps upwards so it
wouldn't do anything. I'm probably the one who originally added it...
* My fix (b14bd1b0b1) for the tabbed child
crash has a side effect where when you close a floating container, focus
is not given to the tiled container again. I've removed my fix and
removed the call to `send_cursor_motion` from `seat_set_focus_warp`. We
should consider calling it from somewhere earlier in the call stack.
The solution used in 073ac425d5 doesn't
work in all cases because the freed instruction might be ahead in the
list, not necessarily behind.
The new solution delays running the queue until after the loop has
finished iterating, thus avoiding the problem completely.
In set_instructions_ready, calling set_instruction_ready may cause any
number of transactions to get applied, which removes them from the list
being iterated. The iteration variables need to be adjusted
accordingly.
The rendering code doesn't use the exclusive input surface at all
anymore to decide to skip rendering of shell surfaces. This fixes
a weird situation in which a client requests exclusive input but
isn't an overlay layer surface.
The renderer also renders all overlay surfaces in this situation,
not just one. This simplifies the code and fixes rendering when
there are more than one overlay surfaces (e.g. for a virtual
keyboard to type the lockscreen password).
This allows to update the title even if the view doesn't commit.
This is useful e.g. when a terminal sets its toplevel title to
the currently running command and when the view isn't visible.
Also does a few other related things:
* Now uses enum wlr_edges instead of our own enum resize_edge
* Now uses wlr_xcursor_get_resize_name and removes our own
find_resize_edge_name
* Renames drag to move for consistency
When interactively resizing some views (eg. Nautilus), new transactions
are added to the queue faster than the client can process them.
Previously, we would wait for the entire queue to be ready before
applying any of them, but in this case the transactions would time out,
giving the client choppy performance.
This changes the queue handling so it applies the transactions up to the
first waiting transaction, without waiting for the entire queue to be
ready.
The directive sets the timeout before an urgent view becomes normal
again after switching to it from another workspace.
Also:
* When an xwayland surface removes the urgent hint while the timer is
active, we now ignore the request. This happens as soon as the view
receives focus, so it was effectively making the timer pointless.
* The timeout is now only applied when switching to it from another
workspace.
This removes the urgency stuff from the commit handler and puts it in a
new set_hints handler instead. This allows the xwayland surface to
become urgent without having to commit (which doesn't happen if it's on
an non-visible workspace).
Introduces a command to manually set urgency, as well as rendering of
urgent views, sending the IPC event, removing urgency after focused for
one second, and matching urgent views via criteria.
Rather than maintain copies of the entire focus stack, this PR
transactionises the focus by introducing two new properties to the
container state and using those when rendering.
* `bool focused` means this container has actual focus. Only one
container should have this equalling true in its current state.
* `struct sway_container *focus_inactive_child` points to the immediate
child that was most recently focused (eg. for tabbed and stacked
containers).
We currently have several ways of setting debug flags, including command
line arguments, environment variables, and compile-time macros. This
replaces the lot with command line flags.
This PR changes the way we handle transactions to a more simple method.
The new method is to mark containers as dirty from low level code
(eg. arranging, or container_destroy, and eventually seat_set_focus),
then call transaction_commit_dirty which picks up those containers and
runs them through a transaction. The old methods of using transactions
(arrange_and_commit, or creating one manually) are now no longer
possible.
The highest-level code (execute_command and view implementation
handlers) will call transaction_commit_dirty, so most other code just
needs to set containers as dirty. This is done by arranging, but can
also be done by calling container_set_dirty.
The title and marks textures would have their height set from the
config's computed max font height, but the textures were not regenerated
when the config's max font height changed which made a gap appear.
Rather than making it regenerate the title textures every time the
config font height was changed, I've changed it to just make the
textures the height of the title itself and fill any gap when rendering.
Also, the title_width and marks_width variables have been renamed to
make it more obvious that they are in output-buffer-local coordinates.
Fixes#1936.
If we set an instruction as ready twice, it decreases the transaction's
num_waiting a second time and applies the transaction earlier than it
should. This no doubt has undesired effects, probably resulting in a use
after free.
Hopefully fixes the first part of #2207.
When you spawn a process with the exec command, sway now notes the
workspace you had focused and the pid of the child process, then assigns
that workspace to the child when its window appears.
Some of this is carried over from sway 0.15, but with some major
refactoring and centralization of state.
That event comes from the toplevel and not the surface, so would cause
a use-after-free on destroy if the toplevel got destroyed first:
==5454==ERROR: AddressSanitizer: heap-use-after-free on address 0x6110001ed198 at pc 0x000000472d10 bp 0x7ffc19070a80 sp 0x7ffc19070a70
WRITE of size 8 at 0x6110001ed198 thread T0
#0 0x472d0f in wl_list_remove ../common/list.c:157
#1 0x42e159 in handle_destroy ../sway/desktop/xdg_shell_v6.c:243
#2 0x7fa9e5b28ce8 in wlr_signal_emit_safe ../util/signal.c:29
#3 0x7fa9e5afd6b1 in destroy_xdg_surface_v6 ../types/xdg_shell_v6/wlr_xdg_surface_v6.c:101
#4 0x7fa9e5d98025 in destroy_resource src/wayland-server.c:688
#5 0x7fa9e5d98091 in wl_resource_destroy src/wayland-server.c:705
#6 0x7fa9e27f103d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
#7 0x7fa9e27f09fe in ffi_call (/lib64/libffi.so.6+0x59fe)
#8 0x7fa9e5d9bf2c (/lib64/libwayland-server.so.0+0xbf2c)
#9 0x7fa9e5d983de in wl_client_connection_data src/wayland-server.c:420
#10 0x7fa9e5d99f01 in wl_event_loop_dispatch src/event-loop.c:641
#11 0x7fa9e5d98601 in wl_display_run src/wayland-server.c:1260
#12 0x40a2f4 in main ../sway/main.c:433
#13 0x7fa9e527318a in __libc_start_main ../csu/libc-start.c:308
#14 0x40b749 in _start (/opt/wayland/bin/sway+0x40b749)
0x6110001ed198 is located 152 bytes inside of 240-byte region [0x6110001ed100,0x6110001ed1f0)
freed by thread T0 here:
#0 0x7fa9e7c89880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fa9e5affce9 in destroy_xdg_toplevel_v6 ../types/xdg_shell_v6/wlr_xdg_toplevel_v6.c:23
#2 0x7fa9e5d98025 in destroy_resource src/wayland-server.c:688
previously allocated by thread T0 here:
#0 0x7fa9e7c89e50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fa9e5b00eea in create_xdg_toplevel_v6 ../types/xdg_shell_v6/wlr_xdg_toplevel_v6.c:427
#2 0x7fa9e27f103d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
The toplevel only notifies the compositor on destroy if it was mapped,
so only listen to events at map time.
A flash of background was happening for two reasons:
1) We were using the xsurface's dimensions to check if the surface is
ready, but these are pending dimensions.
2) In my particular setup, the default geometry of the xsurface does not
intersect any output, which prevented it from receiving a frame done
event. This made the transaction time out and the client would only
redraw once it's been rendered.
The view was configured with the container coordinates.
Although they were right on the first configure, they
changed after a XCB_CONFIGURE_REQUEST, when the
border was already drawn.
Both sway_output and sway_layer_shell listen to wlr's output destroy event,
but sway_layer_shell needs to access into sway_output's data strucure and needs
to be destroyed first.
Resolve this by making sway_layer_shell listen to a new event that happens at
start of sway_output's destroy handler
Fixes this kind of use-after-free:
==1795==ERROR: AddressSanitizer: heap-use-after-free on address 0x612000191ef0 at pc 0x00000048c388 bp 0x7ffe308f0410 sp 0x7ffe308f0400
WRITE of size 8 at 0x612000191ef0 thread T0
#0 0x48c387 in wl_list_remove ../common/list.c:157
#1 0x42196b in handle_destroy ../sway/desktop/layer_shell.c:275
#2 0x7f55cc2549fa in wlr_signal_emit_safe ../util/signal.c:29
#3 0x7f55cc22cf68 in layer_surface_destroy ../types/wlr_layer_shell.c:182
#4 0x7f55cc22d084 in layer_surface_resource_destroy ../types/wlr_layer_shell.c:196
#5 0x7f55cc4ca025 in destroy_resource src/wayland-server.c:688
#6 0x7f55cc4ca091 in wl_resource_destroy src/wayland-server.c:705
#7 0x7f55cc22c3a2 in resource_handle_destroy ../types/wlr_layer_shell.c:18
#8 0x7f55c8ef103d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
#9 0x7f55c8ef09fe in ffi_call (/lib64/libffi.so.6+0x59fe)
#10 0x7f55cc4cdf2c (/lib64/libwayland-server.so.0+0xbf2c)
#11 0x7f55cc4ca3de in wl_client_connection_data src/wayland-server.c:420
#12 0x7f55cc4cbf01 in wl_event_loop_dispatch src/event-loop.c:641
#13 0x7f55cc4ca601 in wl_display_run src/wayland-server.c:1260
#14 0x40bb1e in server_run ../sway/server.c:141
#15 0x40ab2f in main ../sway/main.c:432
#16 0x7f55cb97318a in __libc_start_main ../csu/libc-start.c:308
#17 0x408d29 in _start (/opt/wayland/bin/sway+0x408d29)
0x612000191ef0 is located 48 bytes inside of 312-byte region [0x612000191ec0,0x612000191ff8)
freed by thread T0 here:
#0 0x7f55ce3bb880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x42f1db in handle_destroy ../sway/desktop/output.c:1275
#2 0x7f55cc2549fa in wlr_signal_emit_safe ../util/signal.c:29
#3 0x7f55cc23b4c2 in wlr_output_destroy ../types/wlr_output.c:284
#4 0x7f55cc1ddc20 in xdg_toplevel_handle_close ../backend/wayland/output.c:235
#5 0x7f55c8ef103d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
previously allocated by thread T0 here:
#0 0x7f55ce3bbe50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x42f401 in handle_new_output ../sway/desktop/output.c:1308
#2 0x7f55cc2549fa in wlr_signal_emit_safe ../util/signal.c:29
#3 0x7f55cc1d6cbf in new_output_reemit ../backend/multi/backend.c:113
#4 0x7f55cc2549fa in wlr_signal_emit_safe ../util/signal.c:29
#5 0x7f55cc1deac7 in wlr_wl_output_create ../backend/wayland/output.c:327
#6 0x7f55cc1db353 in backend_start ../backend/wayland/backend.c:55
#7 0x7f55cc1bad55 in wlr_backend_start ../backend/backend.c:35
#8 0x7f55cc1d67a0 in multi_backend_start ../backend/multi/backend.c:24
#9 0x7f55cc1bad55 in wlr_backend_start ../backend/backend.c:35
#10 0x40ba8a in server_run ../sway/server.c:136
#11 0x40ab2f in main ../sway/main.c:432
#12 0x7f55cb97318a in __libc_start_main ../csu/libc-start.c:308
We were freeing the sway_output immediately upon disconnect which left
a dangling pointer in the output's container. It then tried to use the
pointer when the container is freed.
We don't need to store the sway_output in an output's container which is
destroying, so the fix is to set the pointer to NULL and remove the use
in container_free.
Also added an arrange when the output is disconnected for good measure.
Prompts e.g. authentication request from firefox-wayland ought to be
floating.
This is a bit coarse but just fixed size is not enough, here is what
firefox does:
[1285461.363] -> xdg_wm_base@18.get_xdg_surface(new id xdg_surface@68, wl_surface@71)
[1285461.508] -> xdg_surface@68.get_toplevel(new id xdg_toplevel@67)
[1285461.571] -> xdg_toplevel@67.set_parent(xdg_toplevel@37)
[1285461.630] -> xdg_toplevel@67.set_title("Authentication Required")
[1285461.736] -> xdg_toplevel@67.set_app_id("firefox")
...
[1285476.549] xdg_toplevel@67.configure(0, 0, array)
...
[1285502.080] -> xdg_toplevel@67.set_min_size(299, 187)
[1285502.140] -> xdg_toplevel@67.set_max_size(1920, 32767)
This can also be observed with e.g. the open window of gedit
(gedit->open->other documents)
Ryan Dwyer
Drew DeVault
Ian Fan
emersion
Marien Zwart
Armin Preiml
Brian Ashworth
Ryan Dwyer
Brian Ashworth
Pascal Pascher
Pascal Pascher
somdoron
frsfnrrg
russ morris
Konstantin Pospelov
Dominique Martinet
Rostislav Pehlivanov