Fix memory leaks from casting non-trivial values

SanderMertens · Dec 11, 2024 · f86e608 · f86e608
1 parent 99bc073
commit f86e608
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 4 deletions.
diff --git a/distr/flecs.c b/distr/flecs.c
@@ -76088,18 +76088,26 @@ int flecs_expr_cast_visit_fold(
         return 0;
     }
 
-    ecs_meta_cursor_t cur = ecs_meta_cursor(script->world, dst_type, expr->ptr);
+    void *dst_ptr = ecs_value_new(script->world, dst_type);
+
+    ecs_meta_cursor_t cur = ecs_meta_cursor(script->world, dst_type, dst_ptr);
     ecs_value_t value = {
         .type = src_type,
         .ptr = expr->ptr
     };
 
     if (ecs_meta_set_value(&cur, &value)) {
         flecs_expr_visit_error(script, node, "failed to assign value");
+        ecs_value_free(script->world, dst_type, dst_ptr);
         goto error;
     }
 
+    if (expr->ptr != &expr->storage) {
+        ecs_value_free(script->world, expr->node.type, expr->ptr);
+    }
+
     expr->node.type = dst_type;
+    expr->ptr = dst_ptr;
 
     node->expr = NULL; /* Prevent cleanup */
     flecs_visit_fold_replace(script, node_ptr, (ecs_expr_node_t*)expr);

diff --git a/distr/flecs.h b/distr/flecs.h
@@ -265,7 +265,7 @@
  * When enabled, Flecs will use the OS allocator provided in the OS API directly
  * instead of the builtin block allocator. This can decrease memory utilization
  * as memory will be freed more often, at the cost of decreased performance. */
-// #define FLECS_USE_OS_ALLOC
+#define FLECS_USE_OS_ALLOC
 
 /** @def FLECS_ID_DESC_MAX
  * Maximum number of ids to add ecs_entity_desc_t / ecs_bulk_desc_t */

diff --git a/include/flecs.h b/include/flecs.h
@@ -263,7 +263,7 @@
  * When enabled, Flecs will use the OS allocator provided in the OS API directly
  * instead of the builtin block allocator. This can decrease memory utilization
  * as memory will be freed more often, at the cost of decreased performance. */
-// #define FLECS_USE_OS_ALLOC
+#define FLECS_USE_OS_ALLOC
 
 /** @def FLECS_ID_DESC_MAX
  * Maximum number of ids to add ecs_entity_desc_t / ecs_bulk_desc_t */

diff --git a/src/addons/script/expr/visit_fold.c b/src/addons/script/expr/visit_fold.c
@@ -135,18 +135,26 @@ int flecs_expr_cast_visit_fold(
         return 0;
     }
 
-    ecs_meta_cursor_t cur = ecs_meta_cursor(script->world, dst_type, expr->ptr);
+    void *dst_ptr = ecs_value_new(script->world, dst_type);
+
+    ecs_meta_cursor_t cur = ecs_meta_cursor(script->world, dst_type, dst_ptr);
     ecs_value_t value = {
         .type = src_type,
         .ptr = expr->ptr
     };
 
     if (ecs_meta_set_value(&cur, &value)) {
         flecs_expr_visit_error(script, node, "failed to assign value");
+        ecs_value_free(script->world, dst_type, dst_ptr);
         goto error;
     }
 
+    if (expr->ptr != &expr->storage) {
+        ecs_value_free(script->world, expr->node.type, expr->ptr);
+    }
+
     expr->node.type = dst_type;
+    expr->ptr = dst_ptr;
 
     node->expr = NULL; /* Prevent cleanup */
     flecs_visit_fold_replace(script, node_ptr, (ecs_expr_node_t*)expr);

diff --git a/test/script/src/Expr.c b/test/script/src/Expr.c
@@ -1850,41 +1850,47 @@ void Expr_cond_eq_bool_int(void) {
         test_assert(ecs_expr_run(world, "true == 1", &v, NULL) != NULL);
         test_assert(v.type == ecs_id(ecs_bool_t));
         test_assert(*(bool*)v.ptr == true);
+        ecs_value_free(world, v.type, v.ptr);
     }
 
     {
         ecs_value_t v = {0};
         test_assert(ecs_expr_run(world, "true == 2", &v, NULL) != NULL);
         test_assert(v.type == ecs_id(ecs_bool_t));
         test_assert(*(bool*)v.ptr == true);
+        ecs_value_free(world, v.type, v.ptr);
     }
 
     {
         ecs_value_t v = {0};
         test_assert(ecs_expr_run(world, "true == 0", &v, NULL) != NULL);
         test_assert(v.type == ecs_id(ecs_bool_t));
         test_assert(*(bool*)v.ptr == false);
+        ecs_value_free(world, v.type, v.ptr);
     }
 
     {
         ecs_value_t v = {0};
         test_assert(ecs_expr_run(world, "false == 1", &v, NULL) != NULL);
         test_assert(v.type == ecs_id(ecs_bool_t));
         test_assert(*(bool*)v.ptr == false);
+        ecs_value_free(world, v.type, v.ptr);
     }
 
     {
         ecs_value_t v = {0};
         test_assert(ecs_expr_run(world, "false == 2", &v, NULL) != NULL);
         test_assert(v.type == ecs_id(ecs_bool_t));
         test_assert(*(bool*)v.ptr == false);
+        ecs_value_free(world, v.type, v.ptr);
     }
 
     {
         ecs_value_t v = {0};
         test_assert(ecs_expr_run(world, "false == 0", &v, NULL) != NULL);
         test_assert(v.type == ecs_id(ecs_bool_t));
         test_assert(*(bool*)v.ptr == true);
+        ecs_value_free(world, v.type, v.ptr);
     }
 
     ecs_fini(world);