Fix issue where emplace w/observer could return invalid pointer

SanderMertens · Apr 29, 2023 · 2e4c123 · 2e4c123
1 parent 665736e
commit 2e4c123
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<Position>()
+        .event(flecs::OnAdd)
+        .each([](flecs::entity e, Position&) {
+            e.emplace<Velocity>(1.0f, 2.0f);
+        });
+
+    auto e = ecs.entity()
+        .emplace<Position>(10.0f, 20.0f);
+
+    test_assert(e.has<Position>());
+    test_assert(e.has<Velocity>());
+    test_int(e.get<Velocity>()->x, 1);
+    test_int(e.get<Velocity>()->y, 2);
+    test_int(e.get<Position>()->x, 10);
+    test_int(e.get<Position>()->y, 20);
+}
diff --git 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
     },
     {