This is an automated email from the git hooks/post-receive script.
git pushed a commit to branch wl/browser-all
in repository enlightenment.
View the commit online.
commit c5e1ed5368e932297a7d4b4e2afc0de227578545
Author: Cedric BAIL <[email protected]>
AuthorDate: Sun Aug 16 23:06:19 2026 -0600
e_pixmap - a deferred buffer release outlived the pixmap it was deferred on
Brave crashed the compositor about one run in two, always the same way:
#1 wl_signal_emit (wayland-server-core.h:527) l->notify(l, data)
#2 _e_comp_wl_buffer_cb_destroy (e_comp_wl.c:1324)
calling a notify that is a heap address rather than a function - a listener
struct whose memory has been freed and reused. The listener is never the one
that got freed, which is why this lands nowhere near its cause.
The cause is that deferred releases had no teardown. cd3c2d4d4 routed
e_pixmap_resource_set through _e_pixmap_wayland_buffer_release, which is what
makes the deferral fire at all, and this is the other half of it:
* _e_pixmap_free never drained cp->free_buffers. Every buffer still deferred
kept a discarding_pixmap pointing at the pixmap being freed, and when its
client eventually destroyed it _e_pixmap_cb_deferred_buffer_destroy
*wrote* through that pointer. The heap it corrupted was whatever had been
allocated in that space since - a listener, as it turned out.
* _e_pixmap_cb_deferred_buffer_destroy left its own link in the signal list
it had just been called from, on a buffer about to be freed.
* _e_pixmap_wl_buffers_free cleared b->discarding_pixmap *after* a call that
can free b, and released through the wrapper - which looks at
e_comp->rendering again and could put the buffer straight back onto the
list being emptied, or onto one about to be freed with the pixmap. It is
the end of the deferral, so it releases directly.
* _e_pixmap_free also left held_buffer_destroy_listener linked whenever
_e_pixmap_wayland_image_clear took either of its early returns.
Brave went from crashing one run in two to three clean runs out of four with
these; the remaining one is being chased separately and is not this.
Found by test_browser.c driving Brave through maximise, move, resize and
minimise - a synthetic client does not churn buffers and windows fast enough
to expose it, which is the argument for testing against a real browser.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
---
src/bin/e_pixmap.c | 42 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/src/bin/e_pixmap.c b/src/bin/e_pixmap.c
index 16e1461fb..e5ae0baf2 100644
--- a/src/bin/e_pixmap.c
+++ b/src/bin/e_pixmap.c
@@ -79,6 +79,13 @@ _e_pixmap_cb_deferred_buffer_destroy(struct wl_listener *listener, void *data EI
buffer->discarding_pixmap->free_buffers =
eina_list_remove(buffer->discarding_pixmap->free_buffers, buffer);
buffer->discarding_pixmap = NULL;
+ /* Unlink, and say so. The buffer is being destroyed under us and this
+ * listener lives inside it, so leaving the link in place leaves the list it
+ * is in pointing at memory that is about to go. _e_pixmap_wl_buffers_free
+ * also removes this link, and would remove it a second time if the notify
+ * were not cleared here. */
+ wl_list_remove(&buffer->deferred_destroy_listener.link);
+ buffer->deferred_destroy_listener.notify = NULL;
}
static void
@@ -198,12 +205,25 @@ _e_pixmap_wl_buffers_free(E_Pixmap *cp)
{
E_Comp_Wl_Buffer *b;
+ /* This is the end of the deferral, so release rather than going back
+ * through _e_pixmap_wayland_buffer_release - that would look at
+ * e_comp->rendering again and could put the buffer straight back on the
+ * list being emptied, or on a list about to be freed with the pixmap.
+ * Everything below is its non-deferred path, in order.
+ *
+ * Order matters within each buffer too: _e_pixmap_wl_resource_release can
+ * free b - that is what it is for, once the client has destroyed the buffer
+ * we were holding on to - so everything that touches b happens first. */
EINA_LIST_FREE(cp->free_buffers, b)
{
- wl_list_remove(&b->deferred_destroy_listener.link);
- b->deferred_destroy_listener.notify = NULL;
- _e_pixmap_wayland_buffer_release(cp, b);
+ if (b->deferred_destroy_listener.notify)
+ {
+ wl_list_remove(&b->deferred_destroy_listener.link);
+ b->deferred_destroy_listener.notify = NULL;
+ }
b->discarding_pixmap = NULL;
+ if (b->busy == 1) cp->busy_list = eina_list_remove(cp->busy_list, b);
+ _e_pixmap_wl_resource_release(b);
}
}
@@ -266,11 +286,27 @@ _e_pixmap_free(E_Pixmap *cp)
case E_PIXMAP_TYPE_WL:
#ifdef HAVE_WAYLAND
_e_pixmap_wayland_image_clear(cp);
+ /* Anything still deferred belongs to this pixmap, and this pixmap is
+ * about to stop existing. Left alone, each of those buffers keeps a
+ * discarding_pixmap pointing here, and when its client eventually
+ * destroys it _e_pixmap_cb_deferred_buffer_destroy *writes* through
+ * that pointer. The heap it corrupts is the one holding other
+ * listeners, so the crash lands later and somewhere else - in
+ * wl_signal_emit, calling a notify that is no longer a function. */
+ _e_pixmap_wl_buffers_free(cp);
if (cp->buffer_destroy_listener.notify)
{
wl_list_remove(&cp->buffer_destroy_listener.link);
cp->buffer_destroy_listener.notify = NULL;
}
+ /* _e_pixmap_wayland_image_clear only gets here on the path where it
+ * has a held buffer with a pool; on every other path it returns early
+ * and leaves this linked into a signal that outlives the pixmap. */
+ if (cp->held_buffer_destroy_listener.notify)
+ {
+ wl_list_remove(&cp->held_buffer_destroy_listener.link);
+ cp->held_buffer_destroy_listener.notify = NULL;
+ }
#endif
break;
default:
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
