window_destroyed signal fires before window is removed

[dominiklohmann]

Bug report

The event=window_destroyed signal is unreliable, as querying windows from it may or may not include the destroyed window.

This bug has been haunting me for weeks now, making me question my sanity and why signals were unreliable sometimes. I think I've found the root cause:

The signal is transmitted asynchronously before the event is processed internally by yabai. The actual handling of the event happens in the event_destroy function, which is called after event_signal_transmit.

yabai/src/event_loop.c

Lines 60 to 66 in b3eb9d1

	int result = event_handler[event->type](event->context, event->param1, event->param2);
	if (result == EVENT_SUCCESS) event_signal_transmit(event->context, event->type);
	if (event->status) *event->status = EVENT_PROCESSED;
	if (event->result) *event->result = result;
	event_destroy(event);

[koekeishiya]

I don't see a clean solution to this problem as long as the window_destroyed signal transmits both the window id and the window pid.

[dominiklohmann]

Let's rephrase the question then: Iff window_destroyed, application_terminated and display_removed signals were not carrying any arguments, could this issue be resolved?

This might break some scripts that rely on argc=$# or pid=$1, but other than that the data was mostly unusable in practice anyways for these signals.

[koekeishiya]

application_terminated and display_removed doesn't have this problem. I can solve this issue and have window_destroyed pass the window id, just not the application pid.

[dominiklohmann]

I've been wondering whether including the pid is necessary anyways. I've only needed to use it once, but I can get it from the wid anyways with a simple query.

pid="$(yabai -m query --windows --window "${wid}") | jq -r '.pid'" 

[koekeishiya]

Note that that query will not work in your handler for the window_destroyed signal script.

[dominiklohmann]

Yeah, that was for all queries. The pid seems useless outside of the application_* events.

[koekeishiya]

Should we just remove the pid from all window_* signals to be consistent?

[dominiklohmann]

That's a breaking change as it shifts $2 to $1. I'm all for it though. Better break now than later.

[koekeishiya]

Fixed on master.

[dominiklohmann]

Two small things:

• window_created still passed the pid, you only changed the docs for it.
• The ordering issue isn't fixed for window_destroyed, right? Just asking because I don't see it reflected in the code and the issue is labeled addressed not released already.

[koekeishiya]

window_created still passed the pid, you only changed the docs for it.

Thanks for catching that, I was under the impression all window events had their args populated in the same codepath.

The ordering issue isn't fixed for window_destroyed, right?

It's fixed, yes. Windows are now destroyed when the event is handled, before the signal is emitted.