Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Idle inhibit cleanup #1092

Merged
merged 2 commits into from
Jun 28, 2018
Merged

Idle inhibit cleanup #1092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ced9df
wlr_idle_inhibit_v1: add *data pointer to wlr structs
martinetd Jun 28, 2018
93a7576
wlr_idle_inhibit_v1: cleanup destroy handlers
martinetd Jun 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/wlr/types/wlr_idle_inhibit_v1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ struct wlr_idle_inhibit_manager_v1 {
struct {
struct wl_signal new_inhibitor;
} events;

void *data;
};

struct wlr_idle_inhibitor_v1 {
Expand All @@ -37,6 +39,8 @@ struct wlr_idle_inhibitor_v1 {
struct {
struct wl_signal destroy;
} events;

void *data;
};

struct wlr_idle_inhibit_manager_v1 *wlr_idle_inhibit_v1_create(struct wl_display *display);
Expand Down
37 changes: 22 additions & 15 deletions types/wlr_idle_inhibit_v1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,30 @@ wlr_idle_inhibitor_v1_from_resource(struct wl_resource *resource) {
return wl_resource_get_user_data(resource);
}

static void idle_inhibitor_destroy(struct wl_resource *resource) {
struct wlr_idle_inhibitor_v1 *inhibitor =
wlr_idle_inhibitor_v1_from_resource(resource);
static void idle_inhibitor_v1_destroy(struct wlr_idle_inhibitor_v1 *inhibitor) {
if (!inhibitor) {
return;
}

wlr_signal_emit_safe(&inhibitor->events.destroy, inhibitor->surface);

wl_resource_set_user_data(inhibitor->resource, NULL);
wl_list_remove(&inhibitor->link);
wl_list_remove(&inhibitor->surface_destroy.link);
free(inhibitor);
}

static void idle_inhibitor_v1_handle_resource_destroy(struct wl_resource *resource) {
struct wlr_idle_inhibitor_v1 *inhibitor =
wlr_idle_inhibitor_v1_from_resource(resource);
idle_inhibitor_v1_destroy(inhibitor);
}

static void idle_inhibitor_handle_surface_destroy(
struct wl_listener *listener, void *data) {
struct wlr_idle_inhibitor_v1 *inhibitor =
wl_container_of(listener, inhibitor, surface_destroy);

wl_resource_destroy(inhibitor->resource);
idle_inhibitor_v1_destroy(inhibitor);
}

static void idle_inhibitor_v1_handle_destroy(struct wl_client *client,
Expand All @@ -54,7 +61,7 @@ static const struct zwp_idle_inhibitor_v1_interface idle_inhibitor_impl = {
.destroy = idle_inhibitor_v1_handle_destroy,
};

static void manager_create_inhibitor(struct wl_client *client,
static void manager_handle_create_inhibitor(struct wl_client *client,
struct wl_resource *resource, uint32_t id,
struct wl_resource *surface_resource) {
struct wlr_surface *surface = wlr_surface_from_resource(surface_resource);
Expand Down Expand Up @@ -85,25 +92,25 @@ static void manager_create_inhibitor(struct wl_client *client,


wl_resource_set_implementation(wl_resource, &idle_inhibitor_impl,
inhibitor, idle_inhibitor_destroy);
inhibitor, idle_inhibitor_v1_handle_resource_destroy);

wl_list_insert(&manager->inhibitors, &inhibitor->link);
wlr_signal_emit_safe(&manager->events.new_inhibitor, inhibitor);
}


static void idle_inhibit_manager_v1_destroy(struct wl_resource *resource) {
static void idle_inhibit_manager_v1_handle_resource_destroy(
struct wl_resource *resource) {
wl_list_remove(wl_resource_get_link(resource));
}

static void manager_destroy(struct wl_client *client,
static void manager_handle_destroy(struct wl_client *client,
struct wl_resource *manager_resource) {
wl_resource_destroy(manager_resource);
}

static const struct zwp_idle_inhibit_manager_v1_interface idle_inhibit_impl = {
.destroy = manager_destroy,
.create_inhibitor = manager_create_inhibitor,
.destroy = manager_handle_destroy,
.create_inhibitor = manager_handle_create_inhibitor,
};

static void handle_display_destroy(struct wl_listener *listener, void *data) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was reverting the idle inhibit manager to just wl_destroying on external destroy until I stumbled on this function... Many other globals types are the same so I'll let it be for now but I thought I'd bring this up: the compositors are just all supposed to know that if the display gets destroyed they can't access e.g. server->idle_inhibitor anymore?!

It's very perverse: even if the compositor would want to listen to display destroy to clean up its side of the manager properly, it cannot touch the wlr side of things at all because there is no ordering guarantee.

I think any type that can self-destruct like this needs to have a destroy event of its own so the compositor can have a chance to cleanup properly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right - #1096

Expand All @@ -128,7 +135,7 @@ static void idle_inhibit_bind(struct wl_client *wl_client, void *data,
wl_list_insert(&idle_inhibit->wl_resources, wl_resource_get_link(wl_resource));

wl_resource_set_implementation(wl_resource, &idle_inhibit_impl,
idle_inhibit, idle_inhibit_manager_v1_destroy);
idle_inhibit, idle_inhibit_manager_v1_handle_resource_destroy);
wlr_log(L_DEBUG, "idle_inhibit bound");
}

Expand All @@ -142,13 +149,13 @@ void wlr_idle_inhibit_v1_destroy(struct wlr_idle_inhibit_manager_v1 *idle_inhibi
struct wlr_idle_inhibitor_v1 *inhibitor;
struct wlr_idle_inhibitor_v1 *tmp;
wl_list_for_each_safe(inhibitor, tmp, &idle_inhibit->inhibitors, link) {
wl_resource_destroy(inhibitor->resource);
idle_inhibitor_v1_destroy(inhibitor);
}

struct wl_resource *resource;
struct wl_resource *tmp_resource;
wl_resource_for_each_safe(resource, tmp_resource, &idle_inhibit->wl_resources) {
wl_resource_destroy(inhibitor->resource);
wl_resource_destroy(resource);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fun little bug we had before here...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn

}

wl_global_destroy(idle_inhibit->global);
Expand Down