From 198607d10ab8f8fc44540043271d6e3be019250b Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Wed, 7 Feb 2024 08:07:00 +0000 Subject: [PATCH] Fix issue where observer could access deleted id record during world cleanup --- flecs.c | 18 +++++++++++++++--- src/filter.c | 16 ++++++++++++++-- src/search.c | 2 +- test/api/project.json | 3 ++- test/api/src/Event.c | 2 -- test/api/src/OnDelete.c | 29 +++++++++++++++++++++++++++++ test/api/src/main.c | 8 ++++++-- 7 files changed, 67 insertions(+), 11 deletions(-) diff --git a/flecs.c b/flecs.c index 66e2c1097..4e91fdd52 100644 --- a/flecs.c +++ b/flecs.c @@ -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) { @@ -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; @@ -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; diff --git a/src/filter.c b/src/filter.c index 298c398f6..db1d9ee6e 100644 --- a/src/filter.c +++ b/src/filter.c @@ -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) { @@ -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; diff --git a/src/search.c b/src/search.c index 7d57b1580..5539d0487 100644 --- a/src/search.c +++ b/src/search.c @@ -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; diff --git a/test/api/project.json b/test/api/project.json index 8837267a9..fde454f50 100644 --- a/test/api/project.json +++ b/test/api/project.json @@ -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", diff --git a/test/api/src/Event.c b/test/api/src/Event.c index dd14c3ff6..3c1507092 100644 --- a/test/api/src/Event.c +++ b/test/api/src/Event.c @@ -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, diff --git a/test/api/src/OnDelete.c b/test/api/src/OnDelete.c index 811e84537..851511b38 100644 --- a/test/api/src/OnDelete.c +++ b/test/api/src/OnDelete.c @@ -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); +} diff --git a/test/api/src/main.c b/test/api/src/main.c index ae9247b01..6c5c9cb02 100644 --- a/test/api/src/main.c +++ b/test/api/src/main.c @@ -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); @@ -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 } }; @@ -13023,7 +13028,6 @@ bake_test_case StackAlloc_testcases[] = { } }; - static bake_test_suite suites[] = { { "Id", @@ -13155,7 +13159,7 @@ static bake_test_suite suites[] = { "OnDelete", NULL, NULL, - 115, + 116, OnDelete_testcases }, {