mirror of
https://gitlab.freedesktop.org/pipewire/pipewire.git
synced 2025-10-29 05:40:27 -04:00
spa: support: loop: handle cancellation better
Register a pthread cleanup handler to guarantee
that `spa_source::{priv, rmask}` are cleared even
if the thread is cancelled while the loop is dispatching.
This is necessary, otherwise `spa_source::priv` could point
to the stack of the cancelled thread, which will lead to
problems like this later:
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007f846b025be2 in detach_source (source=0x7f845f435f60) at ../spa/plugins/support/loop.c:144
144 e->data = NULL;
[Current thread is 1 (LWP 5274)]
(gdb) p e
$1 = (struct spa_poll_event *) 0x7f845e297820
(gdb) bt
#0 0x00007f846b025be2 in detach_source (source=0x7f845f435f60) at ../spa/plugins/support/loop.c:144
#1 0x00007f846b0276ad in free_source (s=0x7f845f435f60) at ../spa/plugins/support/loop.c:359
#2 0x00007f846b02a453 in loop_destroy_source (object=0x7f845f3af478, source=0x7f845f435f60) at ../spa/plugins/support/loop.c:786
#3 0x00007f846b02a886 in impl_clear (handle=0x7f845f3af478) at ../spa/plugins/support/loop.c:859
#4 0x00007f846b172f40 in unref_handle (handle=0x7f845f3af450) at ../src/pipewire/pipewire.c:211
#5 0x00007f846b173579 in pw_unload_spa_handle (handle=0x7f845f3af478) at ../src/pipewire/pipewire.c:346
#6 0x00007f846b15a761 in pw_loop_destroy (loop=0x7f845f434e30) at ../src/pipewire/loop.c:159
#7 0x00007f846b135d8e in pw_data_loop_destroy (loop=0x7f845f434cb0) at ../src/pipewire/data-loop.c:166
#8 0x00007f846b12c31c in pw_context_destroy (context=0x7f845f41c690) at ../src/pipewire/context.c:485
#9 0x00007f846b3ddf9e in jack_client_close (client=0x7f845f3c1030) at ../pipewire-jack/src/pipewire-jack.c:3481
...
This commit is contained in:
parent
0d51f3b74e
commit
479896279e
2 changed files with 99 additions and 7 deletions
|
|
@ -370,6 +370,24 @@ static inline void process_destroy(struct impl *impl)
|
|||
spa_list_init(&impl->destroy_list);
|
||||
}
|
||||
|
||||
struct cancellation_handler_data {
|
||||
struct spa_poll_event *ep;
|
||||
int ep_count;
|
||||
};
|
||||
|
||||
static void cancellation_handler(void *closure)
|
||||
{
|
||||
const struct cancellation_handler_data *data = closure;
|
||||
|
||||
for (int i = 0; i < data->ep_count; i++) {
|
||||
struct spa_source *s = data->ep[i].data;
|
||||
if (SPA_LIKELY(s)) {
|
||||
s->rmask = 0;
|
||||
s->priv = NULL;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static int loop_iterate(void *object, int timeout)
|
||||
{
|
||||
struct impl *impl = object;
|
||||
|
|
@ -384,6 +402,9 @@ static int loop_iterate(void *object, int timeout)
|
|||
spa_loop_control_hook_after(&impl->hooks_list);
|
||||
impl->polling = false;
|
||||
|
||||
struct cancellation_handler_data cdata = { ep, nfds };
|
||||
pthread_cleanup_push(cancellation_handler, &cdata);
|
||||
|
||||
/* first we set all the rmasks, then call the callbacks. The reason is that
|
||||
* some callback might also want to look at other sources it manages and
|
||||
* can then reset the rmask to suppress the callback */
|
||||
|
|
@ -409,13 +430,7 @@ static int loop_iterate(void *object, int timeout)
|
|||
s->func(s);
|
||||
}
|
||||
|
||||
for (i = 0; i < nfds; i++) {
|
||||
struct spa_source *s = ep[i].data;
|
||||
if (SPA_LIKELY(s)) {
|
||||
s->rmask = 0;
|
||||
s->priv = NULL;
|
||||
}
|
||||
}
|
||||
pthread_cleanup_pop(true);
|
||||
|
||||
return nfds;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -374,6 +374,82 @@ PWTEST(destroy_managed_source_before_dispatch_recurse)
|
|||
return PWTEST_PASS;
|
||||
}
|
||||
|
||||
struct ctwd_data {
|
||||
struct spa_source source;
|
||||
int handler_running_barrier;
|
||||
};
|
||||
|
||||
static void ctwd_event_handler(struct spa_source *source)
|
||||
{
|
||||
struct ctwd_data *data = source->data;
|
||||
|
||||
write_eventfd(data->handler_running_barrier);
|
||||
|
||||
for (;;)
|
||||
pause(); /* the purpose of this is to block the loop */
|
||||
}
|
||||
|
||||
static int ctwd_add_source(struct spa_loop *loop, bool async, uint32_t seq,
|
||||
const void *d, size_t size, void *user_data)
|
||||
{
|
||||
struct ctwd_data *data = user_data;
|
||||
|
||||
pwtest_neg_errno_ok(spa_loop_add_source(loop, &data->source));
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
PWTEST(cancel_thread_while_dispatching)
|
||||
{
|
||||
static const struct spa_dict_item data_loop_props_items[] = {
|
||||
{ "loop.cancel", "true" },
|
||||
};
|
||||
static const struct spa_dict data_loop_props = SPA_DICT_INIT_ARRAY(data_loop_props_items);
|
||||
|
||||
struct ctwd_data data = {
|
||||
.source = {
|
||||
.data = &data,
|
||||
.func = ctwd_event_handler,
|
||||
.mask = SPA_IO_IN,
|
||||
.fd = eventfd(0, 0),
|
||||
},
|
||||
.handler_running_barrier = eventfd(0, 0),
|
||||
};
|
||||
|
||||
pw_init(NULL, NULL);
|
||||
|
||||
struct pw_data_loop *dl = pw_data_loop_new(&data_loop_props);
|
||||
pwtest_ptr_notnull(dl);
|
||||
|
||||
struct pw_loop *l = pw_data_loop_get_loop(dl);
|
||||
pwtest_ptr_notnull(l);
|
||||
|
||||
pwtest_neg_errno_ok(pw_data_loop_start(dl));
|
||||
|
||||
pw_loop_invoke(l, ctwd_add_source, 0, NULL, 0, true, &data);
|
||||
pwtest_ptr_notnull(data.source.loop);
|
||||
|
||||
write_eventfd(data.source.fd);
|
||||
read_eventfd(data.handler_running_barrier);
|
||||
|
||||
pwtest_neg_errno_ok(pw_data_loop_stop(dl));
|
||||
|
||||
/* these are the important checks */
|
||||
pwtest_ptr_null(data.source.priv);
|
||||
pwtest_int_eq(data.source.rmask, UINT32_C(0));
|
||||
|
||||
pw_loop_remove_source(l, &data.source);
|
||||
|
||||
pw_data_loop_destroy(dl);
|
||||
|
||||
close(data.source.fd);
|
||||
close(data.handler_running_barrier);
|
||||
|
||||
pw_deinit();
|
||||
|
||||
return PWTEST_PASS;
|
||||
}
|
||||
|
||||
PWTEST_SUITE(support)
|
||||
{
|
||||
pwtest_add(pwtest_loop_destroy2, PWTEST_NOARG);
|
||||
|
|
@ -381,6 +457,7 @@ PWTEST_SUITE(support)
|
|||
pwtest_add(pwtest_loop_recurse2, PWTEST_NOARG);
|
||||
pwtest_add(destroy_managed_source_before_dispatch, PWTEST_NOARG);
|
||||
pwtest_add(destroy_managed_source_before_dispatch_recurse, PWTEST_NOARG);
|
||||
pwtest_add(cancel_thread_while_dispatching, PWTEST_NOARG);
|
||||
|
||||
return PWTEST_PASS;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue