Ensure symmetric observer is cleaned up before its relationship

SanderMertens · Jan 26, 2023 · b55a883 · b55a883
1 parent 5fc3e45
commit b55a883
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 16 deletions.
diff --git a/flecs.c b/flecs.c
@@ -7767,15 +7767,15 @@ void ecs_delete(
         ecs_flags32_t row_flags = ECS_RECORD_TO_ROW_FLAGS(r->row);
         ecs_table_t *table;
         if (row_flags) {
-            if (row_flags & EcsEntityObservedId) {
-                flecs_on_delete(world, entity, 0, true);
-                flecs_on_delete(world, ecs_pair(entity, EcsWildcard), 0, true);
-            }
             if (row_flags & EcsEntityObservedTarget) {
                 flecs_on_delete(world, ecs_pair(EcsFlag, entity), 0, true);
                 flecs_on_delete(world, ecs_pair(EcsWildcard, entity), 0, true);
                 r->idr = NULL;
             }
+            if (row_flags & EcsEntityObservedId) {
+                flecs_on_delete(world, entity, 0, true);
+                flecs_on_delete(world, ecs_pair(entity, EcsWildcard), 0, true);
+            }
             if (row_flags & EcsEntityObservedAcyclic) {
                 table = r->table;
                 if (table) {
@@ -53973,7 +53973,7 @@ void flecs_register_symmetric(ecs_iter_t *it) {
         /* Create observer that adds the reverse relationship when R(X, Y) is
          * added, or remove the reverse relationship when R(X, Y) is removed. */
         ecs_observer_init(world, &(ecs_observer_desc_t){
-            .entity = ecs_entity(world, {.add = {ecs_childof(EcsFlecsInternals)}}),
+            .entity = ecs_entity(world, {.add = {ecs_childof(r)}}),
             .filter.terms[0] = { .id = ecs_pair(r, EcsWildcard) },
             .callback = flecs_on_symmetric_add_remove,
             .events = {EcsOnAdd, EcsOnRemove}
@@ -55524,8 +55524,12 @@ void flecs_id_record_free(
     ecs_id_t id = idr->id;
 
     flecs_id_record_assert_empty(idr);
-    ecs_assert((world->flags & EcsWorldQuit) || (idr->keep_alive == 0), 
-        ECS_ID_IN_USE, "cannot delete id that is in use");
+
+    if (!(world->flags & EcsWorldQuit)) {
+        /* Id is still in use by a filter, query, rule or observer */
+        ecs_assert((idr->keep_alive == 0), 
+            ECS_ID_IN_USE, "cannot delete id that is queried for");
+    }
 
     if (ECS_IS_PAIR(id)) {
         ecs_entity_t rel = ecs_pair_first(world, id);

diff --git a/src/bootstrap.c b/src/bootstrap.c
@@ -389,7 +389,7 @@ void flecs_register_symmetric(ecs_iter_t *it) {
         /* Create observer that adds the reverse relationship when R(X, Y) is
          * added, or remove the reverse relationship when R(X, Y) is removed. */
         ecs_observer_init(world, &(ecs_observer_desc_t){
-            .entity = ecs_entity(world, {.add = {ecs_childof(EcsFlecsInternals)}}),
+            .entity = ecs_entity(world, {.add = {ecs_childof(r)}}),
             .filter.terms[0] = { .id = ecs_pair(r, EcsWildcard) },
             .callback = flecs_on_symmetric_add_remove,
             .events = {EcsOnAdd, EcsOnRemove}

diff --git a/src/entity.c b/src/entity.c
@@ -2425,15 +2425,15 @@ void ecs_delete(
         ecs_flags32_t row_flags = ECS_RECORD_TO_ROW_FLAGS(r->row);
         ecs_table_t *table;
         if (row_flags) {
-            if (row_flags & EcsEntityObservedId) {
-                flecs_on_delete(world, entity, 0, true);
-                flecs_on_delete(world, ecs_pair(entity, EcsWildcard), 0, true);
-            }
             if (row_flags & EcsEntityObservedTarget) {
                 flecs_on_delete(world, ecs_pair(EcsFlag, entity), 0, true);
                 flecs_on_delete(world, ecs_pair(EcsWildcard, entity), 0, true);
                 r->idr = NULL;
             }
+            if (row_flags & EcsEntityObservedId) {
+                flecs_on_delete(world, entity, 0, true);
+                flecs_on_delete(world, ecs_pair(entity, EcsWildcard), 0, true);
+            }
             if (row_flags & EcsEntityObservedAcyclic) {
                 table = r->table;
                 if (table) {

diff --git a/src/id_record.c b/src/id_record.c
@@ -302,8 +302,12 @@ void flecs_id_record_free(
     ecs_id_t id = idr->id;
 
     flecs_id_record_assert_empty(idr);
-    ecs_assert((world->flags & EcsWorldQuit) || (idr->keep_alive == 0), 
-        ECS_ID_IN_USE, "cannot delete id that is in use");
+
+    if (!(world->flags & EcsWorldQuit)) {
+        /* Id is still in use by a filter, query, rule or observer */
+        ecs_assert((idr->keep_alive == 0), 
+            ECS_ID_IN_USE, "cannot delete id that is queried for");
+    }
 
     if (ECS_IS_PAIR(id)) {
         ecs_entity_t rel = ecs_pair_first(world, id);

diff --git a/test/api/project.json b/test/api/project.json
@@ -693,7 +693,9 @@
                 "create_after_delete_with",
                 "delete_with_inherited_tag",
                 "delete_with_inherited_tag_w_query",
-                "delete_with_inherited_tag_w_observer"
+                "delete_with_inherited_tag_w_observer",
+                "delete_symmetric_relation",
+                "delete_observed_symmetric_relation"
             ]
         }, {
             "id": "Set",

diff --git a/test/api/src/OnDelete.c b/test/api/src/OnDelete.c
@@ -2896,3 +2896,31 @@ void OnDelete_delete_with_inherited_tag_w_observer() {
 
     ecs_fini(world);
 }
+
+void OnDelete_delete_symmetric_relation() {
+    ecs_world_t *world = ecs_mini();
+
+    ecs_entity_t e = ecs_new_id(world);
+    ecs_add_id(world, e, EcsSymmetric);
+    ecs_delete(world, e);
+    test_assert(!ecs_is_alive(world, e));
+
+    // Ensures internal observers are cleaned up before relationship id
+
+    ecs_fini(world);
+}
+
+void OnDelete_delete_observed_symmetric_relation() {
+    ecs_world_t *world = ecs_mini();
+
+    ecs_entity_t e = ecs_new_id(world);
+    ecs_add_id(world, e, EcsTag);
+
+    ecs_add_id(world, e, EcsSymmetric);
+    ecs_delete(world, e);
+    test_assert(!ecs_is_alive(world, e));
+
+    // Ensures internal observers are cleaned up before relationship id
+
+    ecs_fini(world);
+}
diff --git a/test/api/src/main.c b/test/api/src/main.c
@@ -655,6 +655,8 @@ void OnDelete_create_after_delete_with(void);
 void OnDelete_delete_with_inherited_tag(void);
 void OnDelete_delete_with_inherited_tag_w_query(void);
 void OnDelete_delete_with_inherited_tag_w_observer(void);
+void OnDelete_delete_symmetric_relation(void);
+void OnDelete_delete_observed_symmetric_relation(void);
 
 // Testsuite 'Set'
 void Set_set_empty(void);
@@ -4850,6 +4852,14 @@ bake_test_case OnDelete_testcases[] = {
     {
         "delete_with_inherited_tag_w_observer",
         OnDelete_delete_with_inherited_tag_w_observer
+    },
+    {
+        "delete_symmetric_relation",
+        OnDelete_delete_symmetric_relation
+    },
+    {
+        "delete_observed_symmetric_relation",
+        OnDelete_delete_observed_symmetric_relation
     }
 };
 
@@ -11642,7 +11652,7 @@ static bake_test_suite suites[] = {
         "OnDelete",
         NULL,
         NULL,
-        101,
+        103,
         OnDelete_testcases
     },
     {