support: also protect against recursive invocations

Add an extra private field to the source to store the pollevent of
the current iteration. This changes ABI but it seems an embedded source
is not used outside of our own plugins and the unit test doesn't test
this ABI case.

Whenever a source is removed, we can set the data field of the
pollevent to NULL so that it won't be handled in any iteration anymore.

Avoid dispatching the same event multiple times when doing recursive
iterations.

Add some more unit tests for this.

Fixes #2114
This commit is contained in:
Barnabás Pőcze 2022-02-08 17:21:10 +01:00 committed by Wim Taymans
parent 9acae229ab
commit bf886ba209
3 changed files with 151 additions and 16 deletions

View file

@ -66,6 +66,8 @@ struct spa_source {
int fd;
uint32_t mask;
uint32_t rmask;
/* private data for the loop implementer */
void *priv;
};
typedef int (*spa_invoke_func_t) (struct spa_loop *loop,

View file

@ -81,9 +81,6 @@ struct impl {
pthread_t thread;
int enter_count;
struct spa_poll_event dispatching[MAX_EP];
uint32_t n_dispatching;
struct spa_source *wakeup;
int ack_fd;
@ -117,6 +114,7 @@ static int loop_add_source(void *object, struct spa_source *source)
{
struct impl *impl = object;
source->loop = &impl->loop;
source->priv = NULL;
return spa_system_pollfd_add(impl->system, impl->poll_fd, source->fd, source->mask, source);
}
@ -129,10 +127,11 @@ static int loop_update_source(void *object, struct spa_source *source)
static int loop_remove_source(void *object, struct spa_source *source)
{
struct impl *impl = object;
uint32_t i;
for (i = 0; i < impl->n_dispatching; i++) {
if (impl->dispatching[i].data == source)
impl->dispatching[i].data = NULL;
struct spa_poll_event *e;
if ((e = source->priv)) {
/* active in an iteration of the loop, remove it from there */
e->data = NULL;
source->priv = NULL;
}
source->loop = NULL;
return spa_system_pollfd_del(impl->system, impl->poll_fd, source->fd);
@ -329,7 +328,7 @@ static void loop_leave(void *object)
static int loop_iterate(void *object, int timeout)
{
struct impl *impl = object;
struct spa_poll_event *ep = impl->dispatching;
struct spa_poll_event ep[MAX_EP], *e;
int i, nfds;
spa_loop_control_hook_before(&impl->hooks_list);
@ -346,16 +345,20 @@ static int loop_iterate(void *object, int timeout)
* can then reset the rmask to suppress the callback */
for (i = 0; i < nfds; i++) {
struct spa_source *s = ep[i].data;
if (SPA_LIKELY(s))
s->rmask = ep[i].events;
s->rmask = ep[i].events;
/* already active in another iteration of the loop,
* remove it from that iteration */
if (SPA_UNLIKELY(e = s->priv))
e->data = NULL;
s->priv = &ep[i];
}
impl->n_dispatching = nfds;
for (i = 0; i < nfds; i++) {
struct spa_source *s = ep[i].data;
if (SPA_LIKELY(s && s->rmask))
if (SPA_LIKELY(s && s->rmask)) {
s->priv = NULL;
s->func(s);
}
}
impl->n_dispatching = 0;
return nfds;
}

View file

@ -41,6 +41,7 @@ struct data {
struct pw_main_loop *ml;
struct pw_loop *l;
struct obj *a, *b;
int count;
};
static void on_event(struct spa_source *source)
@ -57,13 +58,13 @@ static void on_event(struct spa_source *source)
pw_main_loop_quit(d->ml);
}
PWTEST(pwtest_loop_destroy2)
{
struct data data;
pw_init(0, NULL);
spa_zero(data);
data.ml = pw_main_loop_new(NULL);
pwtest_ptr_notnull(data.ml);
@ -96,9 +97,138 @@ PWTEST(pwtest_loop_destroy2)
return PWTEST_PASS;
}
PWTEST_SUITE(support)
static void
on_event_recurse1(struct spa_source *source)
{
pwtest_add(pwtest_loop_destroy2, PWTEST_NOARG);
static bool first = true;
struct data *d = source->data;
uint64_t val;
++d->count;
pwtest_int_lt(d->count, 3);
read(source->fd, &val, sizeof(val));
if (first) {
first = false;
pw_loop_enter(d->l);
pw_loop_iterate(d->l, -1);
pw_loop_leave(d->l);
}
pw_main_loop_quit(d->ml);
}
PWTEST(pwtest_loop_recurse1)
{
struct data data;
pw_init(0, NULL);
spa_zero(data);
data.ml = pw_main_loop_new(NULL);
pwtest_ptr_notnull(data.ml);
data.l = pw_main_loop_get_loop(data.ml);
pwtest_ptr_notnull(data.l);
data.a = calloc(1, sizeof(*data.a));
data.b = calloc(1, sizeof(*data.b));
data.a->source.func = on_event_recurse1;
data.a->source.fd = eventfd(0, 0);
data.a->source.mask = SPA_IO_IN;
data.a->source.data = &data;
data.b->source.func = on_event_recurse1;
data.b->source.fd = eventfd(0, 0);
data.b->source.mask = SPA_IO_IN;
data.b->source.data = &data;
pw_loop_add_source(data.l, &data.a->source);
pw_loop_add_source(data.l, &data.b->source);
write(data.a->source.fd, &(uint64_t){1}, sizeof(uint64_t));
write(data.b->source.fd, &(uint64_t){1}, sizeof(uint64_t));
pw_main_loop_run(data.ml);
pw_main_loop_destroy(data.ml);
pw_deinit();
return PWTEST_PASS;
}
static void
on_event_recurse2(struct spa_source *source)
{
static bool first = true;
struct data *d = source->data;
uint64_t val;
++d->count;
pwtest_int_lt(d->count, 3);
read(source->fd, &val, sizeof(val));
if (first) {
first = false;
pw_loop_enter(d->l);
pw_loop_iterate(d->l, -1);
pw_loop_leave(d->l);
} else {
pw_loop_remove_source(d->l, &d->a->source);
pw_loop_remove_source(d->l, &d->b->source);
close(d->a->source.fd);
close(d->b->source.fd);
free(d->a);
free(d->b);
}
pw_main_loop_quit(d->ml);
}
PWTEST(pwtest_loop_recurse2)
{
struct data data;
pw_init(0, NULL);
spa_zero(data);
data.ml = pw_main_loop_new(NULL);
pwtest_ptr_notnull(data.ml);
data.l = pw_main_loop_get_loop(data.ml);
pwtest_ptr_notnull(data.l);
data.a = calloc(1, sizeof(*data.a));
data.b = calloc(1, sizeof(*data.b));
data.a->source.func = on_event_recurse2;
data.a->source.fd = eventfd(0, 0);
data.a->source.mask = SPA_IO_IN;
data.a->source.data = &data;
data.b->source.func = on_event_recurse2;
data.b->source.fd = eventfd(0, 0);
data.b->source.mask = SPA_IO_IN;
data.b->source.data = &data;
pw_loop_add_source(data.l, &data.a->source);
pw_loop_add_source(data.l, &data.b->source);
write(data.a->source.fd, &(uint64_t){1}, sizeof(uint64_t));
write(data.b->source.fd, &(uint64_t){1}, sizeof(uint64_t));
pw_main_loop_run(data.ml);
pw_main_loop_destroy(data.ml);
pw_deinit();
return PWTEST_PASS;
}
PWTEST_SUITE(support)
{
pwtest_add(pwtest_loop_destroy2, PWTEST_NOARG);
pwtest_add(pwtest_loop_recurse1, PWTEST_NOARG);
pwtest_add(pwtest_loop_recurse2, PWTEST_NOARG);
return PWTEST_PASS;
}