From 2e4c12341daac8bbcccb2ff9c333d2a9a5169cd5 Mon Sep 17 00:00:00 2001 From: Sander Mertens Date: Sat, 29 Apr 2023 12:33:36 -0700 Subject: [PATCH] Fix issue where emplace w/observer could return invalid pointer --- flecs.c | 3 +-- src/entity.c | 3 +-- test/api/project.json | 3 ++- test/api/src/Set.c | 45 +++++++++++++++++++++++++++++++++++++ test/api/src/main.c | 7 +++++- test/cpp_api/project.json | 3 ++- test/cpp_api/src/Entity.cpp | 20 +++++++++++++++++ test/cpp_api/src/main.cpp | 7 +++++- 8 files changed, 83 insertions(+), 8 deletions(-) diff --git a/flecs.c b/flecs.c index 84ceb8986b..a0dab400ab 100644 --- a/flecs.c +++ b/flecs.c @@ -8409,12 +8409,11 @@ void* ecs_emplace_id( ecs_record_t *r = flecs_entities_get(world, entity); flecs_add_id_w_record(world, entity, r, id, false /* Add without ctor */); + flecs_defer_end(world, stage); void *ptr = flecs_get_component(world, r->table, ECS_RECORD_TO_ROW(r->row), id); ecs_check(ptr != NULL, ECS_INVALID_PARAMETER, NULL); - flecs_defer_end(world, stage); - return ptr; error: return NULL; diff --git a/src/entity.c b/src/entity.c index 10cc810bbc..7a3ec2d921 100644 --- a/src/entity.c +++ b/src/entity.c @@ -2926,12 +2926,11 @@ void* ecs_emplace_id( ecs_record_t *r = flecs_entities_get(world, entity); flecs_add_id_w_record(world, entity, r, id, false /* Add without ctor */); + flecs_defer_end(world, stage); void *ptr = flecs_get_component(world, r->table, ECS_RECORD_TO_ROW(r->row), id); ecs_check(ptr != NULL, ECS_INVALID_PARAMETER, NULL); - flecs_defer_end(world, stage); - return ptr; error: return NULL; diff --git a/test/api/project.json b/test/api/project.json index fea060f8e9..e59d29cd22 100644 --- a/test/api/project.json +++ b/test/api/project.json @@ -811,7 +811,8 @@ "get_mut_w_realloc_in_on_add", "emplace", "emplace_existing", - "emplace_w_move" + "emplace_w_move", + "emplace_w_observer_w_add" ] }, { "id": "ReadWrite", diff --git a/test/api/src/Set.c b/test/api/src/Set.c index 08a220547c..1456c4a789 100644 --- a/test/api/src/Set.c +++ b/test/api/src/Set.c @@ -680,3 +680,48 @@ void Set_emplace_w_move() { ecs_fini(world); } + +static +void OnAddMove(ecs_iter_t *it) { + int i; + for (i = 0; i < it->count; i ++) { + ecs_set(it->world, it->entities[i], Velocity, {1, 2}); + } +} + +void Set_emplace_w_observer_w_add() { + ecs_world_t *world = ecs_mini(); + + ECS_COMPONENT_DEFINE(world, Position); + ECS_COMPONENT_DEFINE(world, Velocity); + + ecs_observer_init(world, &(ecs_observer_desc_t){ + .callback = OnAddMove, + .events = {EcsOnAdd}, + .filter.terms[0].id = ecs_id(Position) + }); + + ecs_entity_t e = ecs_new_id(world); + test_assert(e != 0); + + Position *p = ecs_emplace(world, e, Position); + test_assert(p != NULL); + p->x = 10; + p->y = 20; + + test_assert(ecs_has(world, e, Position)); + + { + const Position *pc = ecs_get(world, e, Position); + test_assert(p == pc); + test_int(pc->x, 10); + test_int(pc->y, 20); + + const Velocity *vc = ecs_get(world, e, Velocity); + test_assert(vc != NULL); + test_int(vc->x, 1); + test_int(vc->y, 2); + } + + ecs_fini(world); +} diff --git a/test/api/src/main.c b/test/api/src/main.c index 613c5dec37..9049ab7af2 100644 --- a/test/api/src/main.c +++ b/test/api/src/main.c @@ -769,6 +769,7 @@ void Set_get_mut_w_realloc_in_on_add(void); void Set_emplace(void); void Set_emplace_existing(void); void Set_emplace_w_move(void); +void Set_emplace_w_observer_w_add(void); // Testsuite 'ReadWrite' void ReadWrite_read(void); @@ -5463,6 +5464,10 @@ bake_test_case Set_testcases[] = { { "emplace_w_move", Set_emplace_w_move + }, + { + "emplace_w_observer_w_add", + Set_emplace_w_observer_w_add } }; @@ -12486,7 +12491,7 @@ static bake_test_suite suites[] = { "Set", NULL, NULL, - 29, + 30, Set_testcases }, { diff --git a/test/cpp_api/project.json b/test/cpp_api/project.json index 30a2e33168..ecf410dd26 100644 --- a/test/cpp_api/project.json +++ b/test/cpp_api/project.json @@ -258,7 +258,8 @@ "get_depth_w_type", "to_view", "to_view_from_stage", - "set_alias" + "set_alias", + "emplace_w_observer" ] }, { "id": "Pairs", diff --git a/test/cpp_api/src/Entity.cpp b/test/cpp_api/src/Entity.cpp index 0c7a50e297..c64944f026 100644 --- a/test/cpp_api/src/Entity.cpp +++ b/test/cpp_api/src/Entity.cpp @@ -4285,3 +4285,23 @@ void Entity_set_alias() { test_assert(e == world.lookup("parent::child")); test_assert(e == world.lookup("parent_child")); } + +void Entity_emplace_w_observer() { + flecs::world ecs; + + ecs.observer() + .event(flecs::OnAdd) + .each([](flecs::entity e, Position&) { + e.emplace(1.0f, 2.0f); + }); + + auto e = ecs.entity() + .emplace(10.0f, 20.0f); + + test_assert(e.has()); + test_assert(e.has()); + test_int(e.get()->x, 1); + test_int(e.get()->y, 2); + test_int(e.get()->x, 10); + test_int(e.get()->y, 20); +} diff --git a/test/cpp_api/src/main.cpp b/test/cpp_api/src/main.cpp index 3f11037da1..7cf0f1ca82 100644 --- a/test/cpp_api/src/main.cpp +++ b/test/cpp_api/src/main.cpp @@ -251,6 +251,7 @@ void Entity_get_depth_w_type(void); void Entity_to_view(void); void Entity_to_view_from_stage(void); void Entity_set_alias(void); +void Entity_emplace_w_observer(void); // Testsuite 'Pairs' void Pairs_add_component_pair(void); @@ -2171,6 +2172,10 @@ bake_test_case Entity_testcases[] = { { "set_alias", Entity_set_alias + }, + { + "emplace_w_observer", + Entity_emplace_w_observer } }; @@ -5876,7 +5881,7 @@ static bake_test_suite suites[] = { "Entity", NULL, NULL, - 238, + 239, Entity_testcases }, {