Skip to content

Commit

Permalink
Fix issue where observer could access deleted id record during world …
Browse files Browse the repository at this point in the history
…cleanup
  • Loading branch information
SanderMertens committed Feb 7, 2024
1 parent 1e7e4b4 commit 198607d
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 11 deletions.
18 changes: 15 additions & 3 deletions flecs.c
Original file line number Diff line number Diff line change
Expand Up @@ -11989,8 +11989,14 @@ bool flecs_term_match_table(
ecs_table_record_t *tr = 0;
bool is_any = is_any_pair(id);

ecs_id_record_t *idr = term->idr;
if (world->flags & EcsWorldQuit) {
/* During world cleanup the keep_alive assert for id records is no
* longer enforced */
idr = NULL;
}
column = flecs_search_relation_w_idr(world, match_table,
column, id, src->trav, src->flags, &source, id_out, &tr, term->idr);
column, id, src->trav, src->flags, &source, id_out, &tr, idr);

if (tr && match_index_out) {
if (!is_any) {
Expand Down Expand Up @@ -12346,8 +12352,14 @@ bool flecs_term_iter_find_superset(
ecs_term_id_t *src = &term->src;

/* Test if following the relationship finds the id */
ecs_id_record_t *idr = term->idr;
if (world->flags & EcsWorldQuit) {
/* During world cleanup the keep_alive assert for id records is no
* longer enforced */
idr = NULL;
}
int32_t index = flecs_search_relation_w_idr(world, table, 0,
term->id, src->trav, src->flags, source, id, 0, term->idr);
term->id, src->trav, src->flags, source, id, 0, idr);

if (index == -1) {
*source = 0;
Expand Down Expand Up @@ -20404,7 +20416,7 @@ int32_t flecs_type_search(
ecs_id_t *ids,
ecs_id_t *id_out,
ecs_table_record_t **tr_out)
{
{
ecs_table_record_t *tr = ecs_table_cache_get(&idr->cache, table);
if (tr) {
int32_t r = tr->index;
Expand Down
16 changes: 14 additions & 2 deletions src/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -2191,8 +2191,14 @@ bool flecs_term_match_table(
ecs_table_record_t *tr = 0;
bool is_any = is_any_pair(id);

ecs_id_record_t *idr = term->idr;
if (world->flags & EcsWorldQuit) {
/* During world cleanup the keep_alive assert for id records is no
* longer enforced */
idr = NULL;
}
column = flecs_search_relation_w_idr(world, match_table,
column, id, src->trav, src->flags, &source, id_out, &tr, term->idr);
column, id, src->trav, src->flags, &source, id_out, &tr, idr);

if (tr && match_index_out) {
if (!is_any) {
Expand Down Expand Up @@ -2548,8 +2554,14 @@ bool flecs_term_iter_find_superset(
ecs_term_id_t *src = &term->src;

/* Test if following the relationship finds the id */
ecs_id_record_t *idr = term->idr;
if (world->flags & EcsWorldQuit) {
/* During world cleanup the keep_alive assert for id records is no
* longer enforced */
idr = NULL;
}
int32_t index = flecs_search_relation_w_idr(world, table, 0,
term->id, src->trav, src->flags, source, id, 0, term->idr);
term->id, src->trav, src->flags, source, id, 0, idr);

if (index == -1) {
*source = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/search.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ int32_t flecs_type_search(
ecs_id_t *ids,
ecs_id_t *id_out,
ecs_table_record_t **tr_out)
{
{
ecs_table_record_t *tr = ecs_table_cache_get(&idr->cache, table);
if (tr) {
int32_t r = tr->index;
Expand Down
3 changes: 2 additions & 1 deletion test/api/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,8 @@
"delete_w_low_rel_mixed_cleanup",
"delete_w_low_rel_mixed_cleanup_interleaved_ids",
"fini_query_w_singleton_in_scope_no_module",
"fini_query_w_singleton_in_module"
"fini_query_w_singleton_in_module",
"fini_observer_w_relationship_in_scope"
]
}, {
"id": "Set",
Expand Down
2 changes: 0 additions & 2 deletions test/api/src/Event.c
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,6 @@ void Event_emit_nested(void) {
.ctx = &ctx_2,
.callback = Nested2
});

printf("\n\n\n\n\n\n");

ecs_emit(world, &(ecs_event_desc_t) {
.event = Event,
Expand Down
29 changes: 29 additions & 0 deletions test/api/src/OnDelete.c
Original file line number Diff line number Diff line change
Expand Up @@ -3166,3 +3166,32 @@ void OnDelete_fini_query_w_singleton_in_module(void) {

ecs_fini(world);
}

void OnDelete_fini_observer_w_relationship_in_scope(void) {
ecs_world_t *world = ecs_mini();

ecs_entity_t Tag = ecs_new_id(world);

ecs_entity_t ns = ecs_new_entity(world, "ns");
ecs_entity_t Rel = ecs_component(world, {
.entity = ecs_entity(world, { .name = "Rel" }),
});

ecs_add_pair(world, Rel, EcsChildOf, ns);

ecs_observer(world, {
.filter.terms = {
{ ecs_pair(Rel, EcsWildcard) },
{ Tag }
},
.events = { EcsOnRemove },
.callback = Observer
});

ecs_new_w_id(world, Tag);

ecs_fini(world);

// Tests edge case where above code would cause a crash
test_assert(true);
}
8 changes: 6 additions & 2 deletions test/api/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ void OnDelete_delete_w_low_rel_mixed_cleanup(void);
void OnDelete_delete_w_low_rel_mixed_cleanup_interleaved_ids(void);
void OnDelete_fini_query_w_singleton_in_scope_no_module(void);
void OnDelete_fini_query_w_singleton_in_module(void);
void OnDelete_fini_observer_w_relationship_in_scope(void);

// Testsuite 'Set'
void Set_set_empty(void);
Expand Down Expand Up @@ -5692,6 +5693,10 @@ bake_test_case OnDelete_testcases[] = {
{
"fini_query_w_singleton_in_module",
OnDelete_fini_query_w_singleton_in_module
},
{
"fini_observer_w_relationship_in_scope",
OnDelete_fini_observer_w_relationship_in_scope
}
};

Expand Down Expand Up @@ -13023,7 +13028,6 @@ bake_test_case StackAlloc_testcases[] = {
}
};


static bake_test_suite suites[] = {
{
"Id",
Expand Down Expand Up @@ -13155,7 +13159,7 @@ static bake_test_suite suites[] = {
"OnDelete",
NULL,
NULL,
115,
116,
OnDelete_testcases
},
{
Expand Down

0 comments on commit 198607d

Please sign in to comment.