On 2017-12-28 01:53 PM, Daniel Stone wrote:
Commit e273c7cde added a refcount to wl_proxy. The refcount is set to 1
on creation, decreased when the client explicitly destroys the proxy,
and is increased and decreased every time an event referencing that
proxy is queued.
Assuming no bugs, this means the refcount cannot reach 0 without the
proxy being explicitly destroyed. However, some (not all) of the
proxy-unref paths were only destroying the proxy if it had already been
deleted. This should already be enforced by refcounting, so remove the
check and rely solely on the refcount as the arbiter of when to free a
proxy.
Signed-off-by: Daniel Stone <dani...@collabora.com>
Cc: Jonas Ådahl <jad...@gmail.com>
Cc: Derek Foreman <der...@osg.samsung.com>
---
src/wayland-client.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 21ef5b04..55838cd9 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -258,7 +258,6 @@ wl_event_queue_release(struct wl_event_queue *queue)
{
struct wl_closure *closure;
struct wl_proxy *proxy;
- bool proxy_destroyed;
while (!wl_list_empty(&queue->event_list)) {
closure = container_of(queue->event_list.next,
@@ -268,10 +267,8 @@ wl_event_queue_release(struct wl_event_queue *queue)
decrease_closure_args_refcount(closure);
proxy = closure->proxy;
- proxy_destroyed = !!(proxy->flags & WL_PROXY_FLAG_DESTROYED);
-
proxy->refcount--;
- if (proxy_destroyed && !proxy->refcount)
+ if (!proxy->refcount)
free(proxy);
wl_closure_destroy(closure);
@@ -1310,10 +1307,10 @@ dispatch_event(struct wl_display *display, struct
wl_event_queue *queue)
proxy_destroyed = !!(proxy->flags & WL_PROXY_FLAG_DESTROYED);
proxy->refcount--;
- if (proxy_destroyed) {
- if (!proxy->refcount)
- free(proxy);
+ if (!proxy->refcount)
+ free(proxy);
This descends into use after free in the case the patch is concerned with?
I see you've turned that into an assert in the follow up patch though,
which looks like a solid idea to me.
This and the follow up are
Reviewed-by: Derek Foreman <der...@osg.samsung.com>
+ if (proxy_destroyed) {
wl_closure_destroy(closure);
return;
}
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
