Segfault when there's a layer surface on a monitor being disconnected #314
Comments
|
Maybe applying this? diff --git a/dwl.c b/dwl.c
index 2e00799..3d88751 100644
--- a/dwl.c
+++ b/dwl.c
@@ -759,16 +759,22 @@ void
cleanupmon(struct wl_listener *listener, void *data)
{
Monitor *m = wl_container_of(listener, m, destroy);
- int nmons, i = 0;
+ LayerSurface *l;
+ int nmons, i;
wl_list_remove(&m->destroy.link);
wl_list_remove(&m->frame.link);
wl_list_remove(&m->link);
m->wlr_output->data = NULL;
+
+ for (i = 0; i >= ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY; i++)
+ wl_list_for_each(l, &m->layers[i], link)
+ wlr_layer_surface_v1_destroy(l->layer_surface);
+
wlr_output_layout_remove(output_layout, m->wlr_output);
wlr_scene_output_destroy(m->scene_output);
- if ((nmons = wl_list_length(&mons)))
+ if ((i = 0) || (nmons = wl_list_length(&mons)))
do /* don't switch to disabled mons */
selmon = wl_container_of(mons.prev, selmon, link);
while (!selmon->wlr_output->enabled && i++ < nmons); |
|
Or this: diff --git a/dwl.c b/dwl.c
index 2e00799..6f85555 100644
--- a/dwl.c
+++ b/dwl.c
@@ -764,6 +764,8 @@ cleanupmon(struct wl_listener *listener, void *data)
wl_list_remove(&m->destroy.link);
wl_list_remove(&m->frame.link);
wl_list_remove(&m->link);
+ for (i = 0; i >= ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY; i++)
+ wl_list_remove(&m->layers[i]);
m->wlr_output->data = NULL;
wlr_output_layout_remove(output_layout, m->wlr_output);
wlr_scene_output_destroy(m->scene_output); |
|
@sev17: What should happen when the last monitor is disconnected? Is this considered undefined behavior? Should clients remain in last known position until a new monitor is attached? Is dwl expected to die in this circumstance? Currently (with layer surface clients in play on all monitors) if I disconnect the only [edit] remaining [/edit] monitor (via a kvm switch), and then return kvm connection seconds or minutes later, dwl is dead and I have to restart. This may be due to this bug listed; I have not tried the proposed patches yet or checked whether dwl dies when no layer shell is present, but it may be intended behavior, so I am asking here instead of starting a new bug. |
|
@sevz17 Neither of those patches seem to fix this (error looks the same in valgrind as well) |
I'm unsure about clients, but definitely I don't want dwl to die
Maybe, lets see if solving this issue fixes it |
BenJarg
changed the title
|
@BenJarg can you apply this and send me the dwl output? diff --git a/dwl.c b/dwl.c
index c8364e9..f21fb32 100644
--- a/dwl.c
+++ b/dwl.c
@@ -2666,6 +2666,7 @@ main(int argc, char *argv[])
/* Wayland requires XDG_RUNTIME_DIR for creating its communications socket */
if (!getenv("XDG_RUNTIME_DIR"))
die("XDG_RUNTIME_DIR must be set");
+ wlr_log_init(WLR_DEBUG, NULL);
setup();
run(startup_cmd);
cleanup(); |
|
Starting from where I opened yambar on the external monitor (calling |
|
this one? diff --git a/dwl.c b/dwl.c
index 2e00799..8abb93b 100644
--- a/dwl.c
+++ b/dwl.c
@@ -2353,6 +2353,8 @@ unmaplayersurfacenotify(struct wl_listener *listener, void *data)
LayerSurface *layersurface = wl_container_of(listener, layersurface, unmap);
layersurface->mapped = 0;
+ wlr_surface_send_leave(layersurface->layer_surface->surface,
+ layersurface->layer_surface->output);
wlr_scene_node_set_enabled(layersurface->scene, 0);
if (layersurface == exclusive_focus)
exclusive_focus = NULL; |
|
@sevz17 Still segfaults unfortunately |
|
The same backtrace? |
|
@sevz17 Yes that's right |
|
@sevz17 The issue is present in both, with the same backtrace as before. |
|
Mmm, then I highly suspect it is something from wlroots, but until I can prove it, let's assume it's our fault. |
|
@BenJarg, I've just merge main into wlroots-next branch, please can you install wlroots-git and see if this can be reproduced on it? |
|
And... if I don't abuse of your help, can you check https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/3740 if wlroots' master branch still segfaults? NOTE: if you can't or not willing to do so, don't worry I appreciate the help so far |
|
@sevz17 Good news, running wlroots-next on the current upstream of wlroots (https://gitlab.freedesktop.org/wlroots/wlroots/-/commit/b7e2a2584e99f0b11d564a5cc9c44fc5ba974a3d) no longer produces a segfault. There is some other (less severe) not ideal behavior though: |
|
Good, less work :) About the layer surfaces, probably we should just destroy them when its monitor is destroyed. |
|
@BenJarg, please can you try this patch in main branch? diff --git a/dwl.c b/dwl.c
index fc8f7b2..9f075b4 100644
--- a/dwl.c
+++ b/dwl.c
@@ -764,6 +764,8 @@ cleanupmon(struct wl_listener *listener, void *data)
Monitor *m = wl_container_of(listener, m, destroy);
int nmons, i = 0;
+ wlr_output_enable(m->wlr_output, 0);
+ wlr_output_commit(m->wlr_output);
wl_list_remove(&m->destroy.link);
wl_list_remove(&m->frame.link);
wl_list_remove(&m->link); |
|
@sevz17 The behavior is the same, and the backtrace also seems to be the same. |
|
Can you give me the output of |
|
@sevz17 Here's the output with your last patch applied on the current main branch (7656569): |
|
The problem is in the third frame, it says that the node is enabled (it should be disabled in diff --git a/dwl.c b/dwl.c
index 29c9eea..198f6e2 100644
--- a/dwl.c
+++ b/dwl.c
@@ -762,8 +762,16 @@ void
cleanupmon(struct wl_listener *listener, void *data)
{
Monitor *m = wl_container_of(listener, m, destroy);
+ LayerSurface *l, *tmp;
int nmons, i = 0;
+ for (i = 0; i <= ZWLR_LAYER_SHELL_V1_LAYER_OVERLAY; i++) {
+ wl_list_for_each_safe(l, tmp, &m->layers[i], link) {
+ wlr_scene_node_set_enabled(l->scene, 0);
+ wlr_layer_surface_v1_destroy(l->layer_surface);
+ }
+ }
+
wl_list_remove(&m->destroy.link);
wl_list_remove(&m->frame.link);
wl_list_remove(&m->link);
@@ -771,7 +779,7 @@ cleanupmon(struct wl_listener *listener, void *data)
wlr_output_layout_remove(output_layout, m->wlr_output);
wlr_scene_output_destroy(m->scene_output);
- if ((nmons = wl_list_length(&mons)))
+ if (!(i = 0) && (nmons = wl_list_length(&mons)))
do /* don't switch to disabled mons */
selmon = wl_container_of(mons.prev, selmon, link);
while (!selmon->wlr_output->enabled && i++ < nmons); |
|
@sevz17 This fixes it! No segfault finally. The instance of yambar on the external monitor is still there when I re-plug in the monitor, but now it keeps its exclusion zone. I'm not sure if this is intended behavior, but it's smoother than when I tried the upstream of wlroots on wlroots-next. |
|
Many thanks for your help!
I think it's because yambar creates another layer surface when the monitor is re-plug in, in any case it is ok |
Info
dwl's commit: b5776e5
wlroots version: 0.15.1
Description
Where I start a instance of yambar on an external monitor, then disconnect it, dwl crashes with a segfault. I can also trigger this by opening rofi (the wayland fork) on the externel monitor, then disconnect the monitor before closing it.
The stacktrace doesn't seem that helpful unfortunately:
In a dump from valgrind, I got an additional error just before that stacktrace that may be more insightful:
I assume
closemon()should be doing something to avoid this.The text was updated successfully, but these errors were encountered: