wayland: fix view_table probe chain corruption on widget reopen

The WINGs view_table (open-addressing hash, size 256) used id=0 as both
"empty" and "deleted". Zeroing a slot on removal broke probe chains for
entries that hashed past that slot. After two dialog open/close cycles
(~40 views created and destroyed), enough chains were broken that
W_GetViewForWindow/W_GetViewBacking returned NULL for valid IDs.

Fix: use a tombstone marker (id=1) for deleted slots. Lookups skip
tombstones (continue probing), inserts reuse them. This is the standard
approach for deletion in open-addressing hash tables (Knuth Vol.3 §6.4).

Also:
- wl_destroy now calls view_table_remove + frees backing pixman image
- wl_frame_destroy cleans up child frame_bufs
- client_reparent existing path re-links backing and recreates scene_buf
- client_reparent to root destroys the child frame_buf
This commit is contained in:
lkn 2026-06-22 00:21:42 +02:00
parent 9f32dc4eef
commit 143b8906bb
3 changed files with 76 additions and 9 deletions

View File

@ -49,6 +49,7 @@ extern void wl_view_destroy_backing_impl(unsigned long id);
/* Simple view lookup table (ID → W_View*) */
#define VIEW_TABLE_SIZE 256
#define VIEW_TABLE_TOMBSTONE ((WNativeWindow)1) /* never a valid view/fb ID */
static struct { WNativeWindow id; W_View *view; void *backing; } view_table[VIEW_TABLE_SIZE];
static void view_table_put(WNativeWindow id, W_View *view)
@ -56,7 +57,8 @@ static void view_table_put(WNativeWindow id, W_View *view)
unsigned idx = ((unsigned)id) % VIEW_TABLE_SIZE;
for (int i = 0; i < VIEW_TABLE_SIZE; i++) {
unsigned slot = (idx + i) % VIEW_TABLE_SIZE;
if (view_table[slot].id == 0 || view_table[slot].id == id) {
if (view_table[slot].id == 0 || view_table[slot].id == id
|| view_table[slot].id == VIEW_TABLE_TOMBSTONE) {
view_table[slot].id = id;
view_table[slot].view = view;
return;
@ -71,10 +73,9 @@ W_View *W_GetViewForWindow(WNativeWindow id)
unsigned slot = (idx + i) % VIEW_TABLE_SIZE;
if (view_table[slot].id == id)
return view_table[slot].view;
if (view_table[slot].id == 0) {
//wwarning("W_GetViewForWindow: id=0x%lx NOT FOUND (slot %u empty)", (unsigned long)id, slot);
if (view_table[slot].id == 0)
return NULL;
}
/* skip tombstones */
}
return NULL;
}
@ -113,7 +114,7 @@ void view_table_remove(WNativeWindow id)
for (int i = 0; i < VIEW_TABLE_SIZE; i++) {
unsigned slot = (idx + i) % VIEW_TABLE_SIZE;
if (view_table[slot].id == id) {
view_table[slot].id = 0;
view_table[slot].id = VIEW_TABLE_TOMBSTONE;
view_table[slot].view = NULL;
view_table[slot].backing = NULL;
return;
@ -470,6 +471,11 @@ static void wl_destroy(void *vctx, W_View *view)
W_WLClientToplevelDestroyFn(view);
if (view->flags.topLevel && W_ViewDestroyBackingFn)
W_ViewDestroyBackingFn((unsigned long)view->window);
/* Free backing image and remove from view table */
pixman_image_t *img = (pixman_image_t *)W_GetViewBacking(view->window);
if (img)
pixman_image_unref(img);
view_table_remove(view->window);
}
static void wl_set_background_color(void *vctx, W_View *view, WMColor *color)

View File

@ -93,6 +93,20 @@ wl_client_reparent(WNativeWindow client, WNativeWindow new_parent, int x, int y)
/* Reparent to root = dialog teardown: destroy the child frame_buf */
if (client >= 0x20000000 && new_parent == (WNativeWindow)wScreenWithNumber(0)->root_win) {
struct wl_frame_buf *fb = frame_buf_find(client);
if (fb) {
extern void W_UnregisterBacking(unsigned long id);
W_UnregisterBacking(fb->id);
if (fb->scene_buf)
wlr_scene_node_destroy(&fb->scene_buf->node);
wl_list_remove(&fb->link);
wlr_buffer_drop(&fb->base);
}
return;
}
/* For WINGs internal windows: create a child frame_buf so the dialog
* content is visible in the scene graph. */
if (client >= 0x20000000 && new_parent >= 0x10000000) {
@ -103,15 +117,48 @@ wl_client_reparent(WNativeWindow client, WNativeWindow new_parent, int x, int y)
/* Check if we already have a frame_buf for this client */
struct wl_frame_buf *existing = frame_buf_find(client);
if (existing) {
/* Just update parent and position */
/* Update parent, position, and ensure backing is current */
existing->parent_id = new_parent;
existing->rel_x = x;
existing->rel_y = y;
existing->x = parent_fb->x + x;
existing->y = parent_fb->y + y;
/* Resize backing image if the view size changed */
extern W_View *W_GetViewForWindow(unsigned long id);
W_View *view = W_GetViewForWindow((unsigned long)client);
int w = view ? view->size.width : existing->width;
int h = view ? view->size.height : existing->height;
if (w <= 0) w = 1;
if (h <= 0) h = 1;
if (w != existing->width || h != existing->height) {
pixman_image_unref(existing->image);
existing->image = pixman_image_create_bits(PIXMAN_a8r8g8b8, w, h, NULL, 0);
existing->width = w;
existing->height = h;
existing->base.width = w;
existing->base.height = h;
if (existing->scene_buf)
wlr_scene_buffer_set_dest_size(existing->scene_buf, w, h);
}
/* Re-link the view's backing to this frame_buf's image */
extern void W_SetViewBacking(unsigned long id, void *img);
W_SetViewBacking((unsigned long)client, existing->image);
/* Recreate scene_buf if it was destroyed (cascade from parent tree) */
if (!existing->scene_buf && wl_state.scene) {
struct wlr_scene_tree *_wp = parent_fb->tree
? parent_fb->tree
: (wl_state.level_trees[WL_LAYER_FLOATING]
? wl_state.level_trees[WL_LAYER_FLOATING]
: &wl_state.scene->tree);
existing->scene_buf = wlr_scene_buffer_create(_wp, &existing->base);
if (existing->scene_buf)
fb_attach_scene_buf_listener(existing);
}
if (existing->scene_buf) {
/* Ensure it's under the parent frame's tree, then position
* at relative offset within the tree. */
if (parent_fb->tree &&
existing->scene_buf->node.parent != parent_fb->tree) {
wlr_scene_node_reparent(&existing->scene_buf->node,
@ -122,6 +169,7 @@ wl_client_reparent(WNativeWindow client, WNativeWindow new_parent, int x, int y)
else
wlr_scene_node_set_position(&existing->scene_buf->node,
existing->x, existing->y);
wlr_scene_node_set_enabled(&existing->scene_buf->node, false);
}
return;
}
@ -216,7 +264,7 @@ void wl_view_commit_backing_impl(unsigned long id)
if (fb) {
extern void *W_GetViewBacking(unsigned long id);
void *current = W_GetViewBacking((unsigned long)id);
if (current != (void *)fb->image) {
if (current && current != (void *)fb->image && !fb->parent_id) {
/* Save parent relation before destroying — the new frame_buf
* must be positioned within the same parent tree so dialog
* content reparented via wl_client_reparent stays visible. */

View File

@ -456,6 +456,19 @@ wl_frame_destroy(WScreen *scr, WNativeWindow win)
(void)scr;
struct wl_frame_buf *fb = frame_buf_find(win);
if (!fb) return;
/* Destroy child frame_bufs whose parent is this one */
struct wl_frame_buf *child, *tmp;
wl_list_for_each_safe(child, tmp, &wl_state.frame_list, link) {
if (child->parent_id == win) {
W_UnregisterBacking(child->id);
if (child->scene_buf)
wlr_scene_node_destroy(&child->scene_buf->node);
wl_list_remove(&child->link);
wlr_buffer_drop(&child->base);
}
}
W_UnregisterBacking(fb->id);
/* Destroy the scene_buf and (if any) the tree. Without destroying
* the tree, leaked tree nodes accumulate in the scene graph and