spa: support: loop: do not call control hooks on blocking invoke

The control hooks of a loop are called before the loop starts polling
and after it has finished polling. Currently, this is used to implement
the locking in pw_thread_loop. This is used to guarantee that the thread
loop's lock is taken while the thread loop is dispatching, and that
the lock can be taken while the loop is polling, when it is running
no user-space code.

However, calling the thread control hooks of thread A when doing an
blocking invoke from thread B serves little purpose, and in fact
can cause issues: for example, issuing a blocking invoke on a
pw_thread_loop does not work unless the lock thereof is taken.

This behaviour, of calling the control hooks from other threads,
is also not documented, and goes contrary to what is currently
stated in the loop.h header file:

  /** Executed right before waiting for events. It is typically used to
   * release locks. */
  ...
  /** Executed right after waiting for events. It is typically used to
   * reacquire locks. */

At the moment the implementation allows any thread to queue invoke
items on any other thread without restrictions; calling the control
hooks only places extra restrictions on the usability of this mechanism
(in case of pw_thread_loop, having to take the loop's lock).
So do not call the control hooks when doing a blocking invoke.
This commit is contained in:
Barnabás Pőcze 2024-08-05 16:27:18 +02:00 committed by Wim Taymans
parent 32956efbf7
commit 5fed160972

View file

@ -316,14 +316,10 @@ retry:
if (block) { if (block) {
uint64_t count = 1; uint64_t count = 1;
spa_loop_control_hook_before(&impl->hooks_list);
if ((res = spa_system_eventfd_read(impl->system, queue->ack_fd, &count)) < 0) if ((res = spa_system_eventfd_read(impl->system, queue->ack_fd, &count)) < 0)
spa_log_warn(impl->log, "%p: failed to read event fd:%d: %s", spa_log_warn(impl->log, "%p: failed to read event fd:%d: %s",
queue, queue->ack_fd, spa_strerror(res)); queue, queue->ack_fd, spa_strerror(res));
spa_loop_control_hook_after(&impl->hooks_list);
res = item->res; res = item->res;
} }
else { else {