Mir fails to close when XWayland app is running #1169
Assignees
Labels
Comments
|
It appears this was introduced somewhere in #1160, although Mir was fatal erroring previously. Neither of those behaviors are preferable. |
|
Actually, if you wait, it still crashes. The new, incorrect, behavior is to make several (failing) attempts to restart the Xwayland server. |
This was referenced Jan 8, 2020
AlanGriffiths
added a commit
that referenced
this issue
Jan 13, 2020
1177: Don't restart Xwayland if we killed it deliberately r=wmww a=AlanGriffiths Don't restart Xwayland if we killed it deliberately. (Fixes: #1169) This whole area is a mess, as the reason we kill Xwayland is that it is waiting for clients to exit before closing. And if we ensured they were closed, we wouldn't be deleting windows that no longer exist and crashing after this. I'll have a look at that (fixed, second commit). We still fail to give clients a chance to close nicely, c.f. #1171. Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
AlanGriffiths
added a commit
that referenced
this issue
Jan 16, 2020
* Merge #1113 #1116 1113: Add Surface::set_window_margins() for SSD r=AlanGriffiths a=wmww This allows setting a surface's "window margins" (the padding around a surface that makes room for decorations). There are alternative ways we could have done this, each with its advantages and disadvantages. We settled on this approach several weeks ago because it's easy to implement with minimum code churn, and appears to be similar to what whoever was here last intended to do. In the future (especially if we split up the `scene::Surface` interface) we may want to refactor this, but it has proven to be sufficient for usable SSDs. 1116: Add and remove test files r=AlanGriffiths a=wmww Replace and unused frontend shell stub with a scene shell stub that I will actually need for SSD tests. Also drop an unused mock shell and move `ExplicitExecutor` to its own file as it will be needed in multiple test files. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1117 1117: Add decoration::Manager r=AlanGriffiths a=wmww Adds a new `mir::shell::decoration` namespace, and a `decoration::Manager` configuration global. Stacked on #1116. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1119 1119: SSD surface creation param r=AlanGriffiths a=wmww This lets the XWayland frontend mark its windows as server-side-decorated and causes the shell to decorate them. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1123 1123: BasicSurface: don't allow zero content size r=AlanGriffiths a=wmww Fixes small mistake in implementation of `BasicSurface::content_size()` that may have caused some SSD crashes. EDIT: doesn't look like the old behavior was causing any problem in practice, but this doesn't hurt. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1126 1126: Simplifiy mtd::StubSurface r=RAOF a=wmww Remove a redundant surface stub, and move the method implementations of the remaining one into the header. This reduces the number of files that need to be edited for `scene::Surface` changes by two, which speeds up adding surface properties and future refactoring of the class. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1121 1121: SSD MVP r=AlanGriffiths a=wmww Not based on other PRs, but won't take effect until combined with #1119. After that decorations should appear on all X11 windows. Adds a working title bar with resize edges and minimize/maximize/close buttons. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1129 1129: Correctly constrain window size r=wmww a=wmww Max and min window size were only being respected when first set. This PR constrains the window size whenever the window is resized. Tests included. EDIT: note that this PR alone will not fix SSD size constraints, but it will fix the visible problem of windows moving weirdly when you resize them too small. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1134 1134: [wayland-platform] Apply output scale r=wmww a=AlanGriffiths Apply output scale. (Fixes: #1133) Mir uses unscaled coordinates, so we need to mark buffers as pre-scaled and scale some input values. Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1132 1132: Revert "Merge #1089" r=RAOF a=AlanGriffiths We're no longer seeing intermittent failures to set up ubuntu-devel. Reinstate in CI. This reverts commit a9a7bd5, reversing changes made to 5fdc447. Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1137 1137: x11 tweaks r=wmww a=AlanGriffiths Drop some code used in early testing. Add a configuration option to specify the Xwayland executable. Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1143 1143: MinimalWindowManager: drag the requested window r=AlanGriffiths a=wmww ...instead of whatever is under the cursor (fixes #1135) I believe this is correct implementation because it mirrors what's already in `apply_resize_by()` and `handle_touch_event()`. This should fix a variety of edge cases I've been stumbling on for a while (such as dragging a window under a panel). Co-authored-by: William Wold <wm@wmww.sh> * Merge #1138 1138: Refactor decoration r=AlanGriffiths a=wmww * Add a few properties to the `StaticGeometry` struct that were previously magic numbers strewn about the code * Make `static_geometry` a shared pointer and give all decoration components that need it a copy (instead of having to request it from a `WindowState` * Give `Renderer` a new `Theme` struct that can be changed when switching between the focused and unfocused look A PR adding title text rendering is to come after this is merged. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1145 1145: SSD: Redraw titlebar when buttons move r=AlanGriffiths a=wmww Fixes #1142 Co-authored-by: William Wold <wm@wmww.sh> * Merge #1144 1144: Draw window titles on SSDs r=wmww a=wmww Uses FreeType to render window titles on server side decorations. It appears the current XWayland implementation does not update the window title after it is initially set, but this will handle title updates when it happens. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1150 1150: Draw SSD titlebar button icons r=AlanGriffiths a=wmww Programmatically draws close, minimize and maximize icons. They should look like so:  Co-authored-by: William Wold <wm@wmww.sh> * Merge #1151 1151: XWayland refactor and fix r=AlanGriffiths a=wmww Previously, `XWaylandServer::spawn()` forked, and handled both forks in a large switch statement. This splits the two branches into `XWaylandServer::execl_xwayland()` and `XWaylandServer::connect_to_xwayland()`. Also runs `wl_client_create()` on the Wayland thread to fix #1147. It may be helpful to look over the commits individually. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1146 1146: SSD: improve how BasicDecoration states are updated r=AlanGriffiths a=wmww TLDR this cleans up logic and prevents a flicker. You don't need to fully understand the problem unless you really care. A call to `window_state_updated()` would trigger creating multiple `WindowState`s, which could be different if a change to the window was in-progress. The end result would be correct once all the callbacks had fired, but it would render one frame where the `WindowState` the `InputState` was generated from was different from the one being rendered. This would cause a flicker of button positions. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1152 1152: Tidy up x11 frontend r=wmww a=AlanGriffiths Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1149 1149: Fix process snap version check r=Saviq a=AlanGriffiths We want to update "edge" channels when master has a different version. E.g. >>> "1.6.0+dev73".startswith("1.6.0") True >>> "1.6.0+dev73" =="1.6.0" False Co-authored-by: Alan Griffiths <alan@octopull.co.uk> Co-authored-by: Michał Sawicz <michal@sawicz.net> * Merge #1159 1159: Remove unused function r=wmww a=AlanGriffiths Remove unused function (that cannot be used safely). Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1154 1154: [X11 frontend] Make Wayland calls on the correct thread r=wmww a=AlanGriffiths Make Wayland calls on the correct thread. (Fixes: #1153) Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1162 1162: [server/input] Use a sorted map for outputs r=wmww a=mariogrip This changes the map of outputs from a unordered map to a sorted map, as mir takes the first output as default for touch screens if not otherwise is provided. it makes no sense for this to give a different result depending on whats first in the unordered_map bucket. Co-authored-by: Marius Gripsgard <marius@ubports.com> * Merge #1161 1161: Add frontend::SurfaceStack r=wmww a=wmww Replaces #1155 Co-authored-by: William Wold <wm@wmww.sh> * Merge #1165 1165: [packaging] Add libwayland-dev to Build-Depends r=AlanGriffiths a=RAOF Why did we get away with not having this before? Co-authored-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com> * Merge #1160 1160: [X11 frontend] A pass at tidying XWaylandServer r=wmww a=AlanGriffiths A pass at tidying XWaylandServer: o Reducing scope of stuff o Avoiding indirect recursion o Simplify threading o Initialize on creation & make const Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1170 1170: Fix clang found clangers r=AlanGriffiths a=RAOF Clang 10 picks up a couple of extra warnings, which have picked up some incorrect Mir code. Co-authored-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com> * Merge #1168 1168: Maximize/restore windows with SSDs on titlebar double click r=AlanGriffiths a=wmww Fixes #1167 Double click ssd maximize Co-authored-by: William Wold <wm@wmww.sh> * Merge #1177 1177: Don't restart Xwayland if we killed it deliberately r=wmww a=AlanGriffiths Don't restart Xwayland if we killed it deliberately. (Fixes: #1169) This whole area is a mess, as the reason we kill Xwayland is that it is waiting for clients to exit before closing. And if we ensured they were closed, we wouldn't be deleting windows that no longer exist and crashing after this. I'll have a look at that (fixed, second commit). We still fail to give clients a chance to close nicely, c.f. #1171. Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1176 1176: Some initial XWayland cleanup r=AlanGriffiths a=wmww Removes a lot of dead code that we wont need, moves some stuff around, and patches initial window properties closer to where they will be used. This shouldn't have any noticeable effects on behavior ~except for the first commit, which is the same as #1173 and fixes X #1172~ Co-authored-by: William Wold <wm@wmww.sh> * Merge #1179 1179: Threadsafe XwaylandServer r=wmww a=AlanGriffiths Threadsafe XwaylandServer. Protects the mutable state with a mutex and simplifies formerly racy logic. Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1181 1181: X11: Tidy XWaylandWM r=wmww a=AlanGriffiths More initialize-on-construction, constification and other tidying Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1173 #1182 1173: XWayland: create scene surface only when needed r=AlanGriffiths a=wmww XWayland: create scene surface only when needed. (Fixes: #1172) 1182: Wayland: don't assume PixelSource::read() will execute the do_with_pixels functor r=wmww a=AlanGriffiths Wayland: (cursor code) don't assume PixelSource::read() will execute the do_with_pixels functor. (Fixes: #1180) Co-authored-by: William Wold <wm@wmww.sh> Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1183 1183: XWayland: make names more verbose r=AlanGriffiths a=wmww Also moves some logic from `XWaylandWMSurface` into `XWaylandWM` that removes the need for a few getters. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1184 1184: Drop unused headers r=wmww a=AlanGriffiths Drop unused headers Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Merge #1186 1186: Make XWaylandWMSurface thread safe r=wmww a=wmww Make Wayland calls on the Wayland thread and lock everything else behind a mutext Co-authored-by: William Wold <wm@wmww.sh> * Merge #1156 1156: BasicWindowManager requests on unknown windows should noop r=RAOF a=wmww It is impossible for one subsystem in Mir to know if another subsystem on another thread is in the process of destroying a surface. It is also impossible to externally lock the window manager. Therefore, a surface may be destroyed twice in quick succession, or the window manager may get a `modify_surface` after the surface has been destroyed. The current behavior is to `fatal_error()` if a surface is destroyed twice, and throw `std::out_of_range` or do nothing in other cases. This PR implements, documents and tests: Requests on unknown windows warn and noop. Co-authored-by: William Wold <wm@wmww.sh> * Merge #1191 1191: X11: Don't stop Xwayland when pausing the Mir server r=wmww a=AlanGriffiths We don't need to stop Xwayland when pausing the Mir server. (Fixes: #1188) Co-authored-by: Alan Griffiths <alan@octopull.co.uk> * Bump mirserver ABI * Changelog * Merge #1194 1194: SurfaceCreationParameters::update_from(): correctly access name r=RAOF a=wmww Previously, C++ was helpfully casting the optional into a bool, then a char, then "\x1". Co-authored-by: William Wold <wm@wmww.sh> * Merge #1192 1192: Add top_left to shell::SurfaceSpecification r=RAOF a=wmww It appears this was never added because no mirclient or Wayland protocols needed it. XWayland will. Co-authored-by: William Wold <wm@wmww.sh> * Revert "Merge #1129" (#1202) * Revert "miral: Add window resize tests" This reverts commit abab614 * Revert "BasicWindowManager::modify_window(): respect size constraints" This reverts commit e14dd13 (cherry picked from commit 514352b) * Merge #1198 1198: XWayland: improve verbose logging r=AlanGriffiths a=wmww Improvements to the verbose logs generated by the XWayland frontend. Probably not interesting to anyone but me, but merging will make maintaining my branches simpler. Co-authored-by: William Wold <wm@wmww.sh> Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com> Co-authored-by: William Wold <wm@wmww.sh> Co-authored-by: Michał Sawicz <michal@sawicz.net> Co-authored-by: Marius Gripsgard <marius@ubports.com> Co-authored-by: Christopher James Halse Rogers <chris@cooperteam.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
To reproduce:
mir_demo_serverwith a--x11-display-experimental(only tested with Mir-on-X)Note that there isn't a problem if you close the X11 app before attempting to close Mir.
The text was updated successfully, but these errors were encountered: