Skip to content

Commit

Permalink
Fix double free issue in C++ when iterating terms with allocated reso…
Browse files Browse the repository at this point in the history
…urces
  • Loading branch information
SanderMertens committed Sep 15, 2023
1 parent 7f9d203 commit 49478e7
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 5 deletions.
2 changes: 2 additions & 0 deletions flecs.h
Original file line number Diff line number Diff line change
Expand Up @@ -27512,6 +27512,7 @@ struct filter_base {
for (int i = 0; i < m_filter_ptr->term_count; i ++) {
flecs::term t(m_world, m_filter_ptr->terms[i]);
func(t);
t.reset(); // prevent freeing resources
}
}

Expand Down Expand Up @@ -28001,6 +28002,7 @@ struct query_base {
for (int i = 0; i < f->term_count; i ++) {
flecs::term t(m_world, f->terms[i]);
func(t);
t.reset(); // prevent freeing resources
}
}

Expand Down
1 change: 1 addition & 0 deletions include/flecs/addons/cpp/mixins/filter/impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ struct filter_base {
for (int i = 0; i < m_filter_ptr->term_count; i ++) {
flecs::term t(m_world, m_filter_ptr->terms[i]);
func(t);
t.reset(); // prevent freeing resources
}
}

Expand Down
1 change: 1 addition & 0 deletions include/flecs/addons/cpp/mixins/query/impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ struct query_base {
for (int i = 0; i < f->term_count; i ++) {
flecs::term t(m_world, f->terms[i]);
func(t);
t.reset(); // prevent freeing resources
}
}

Expand Down
7 changes: 5 additions & 2 deletions test/cpp_api/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@
"test_no_defer_iter",
"inspect_terms",
"inspect_terms_w_each",
"inspect_terms_w_expr",
"comp_to_str",
"pair_to_str",
"oper_not_to_str",
Expand Down Expand Up @@ -806,7 +807,8 @@
"is_valid",
"unresolved_by_name",
"scope",
"iter_w_stage"
"iter_w_stage",
"inspect_terms_w_expr"
]
}, {
"id": "SystemBuilder",
Expand Down Expand Up @@ -892,7 +894,8 @@
"invalid_each_w_no_this",
"named_filter",
"named_scoped_filter",
"set_this_var"
"set_this_var",
"inspect_terms_w_expr"
]
}, {
"id": "ComponentLifecycle",
Expand Down
16 changes: 16 additions & 0 deletions test/cpp_api/src/Filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,3 +456,19 @@ void Filter_set_this_var(void) {
});
test_int(count, 1);
}

void Filter_inspect_terms_w_expr(void) {
flecs::world ecs;

flecs::filter<> f = ecs.filter_builder()
.expr("(ChildOf,0)")
.build();

int32_t count = 0;
f.each_term([&](flecs::term &term) {
test_assert(term.id().is_pair());
count ++;
});

test_int(count, 1);
}
15 changes: 15 additions & 0 deletions test/cpp_api/src/Query.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,21 @@ void Query_inspect_terms_w_each(void) {
test_int(count, 3);
}

void Query_inspect_terms_w_expr(void) {
flecs::world ecs;

flecs::filter<> f = ecs.filter_builder()
.expr("(ChildOf,0)")
.build();

int32_t count = 0;
f.each_term([&](flecs::term &term) {
test_assert(term.id().is_pair());
count ++;
});

test_int(count, 1);
}

void Query_comp_to_str(void) {
flecs::world ecs;
Expand Down
16 changes: 16 additions & 0 deletions test/cpp_api/src/RuleBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -797,3 +797,19 @@ void RuleBuilder_iter_w_stage(void) {

q.destruct();
}

void RuleBuilder_inspect_terms_w_expr(void) {
flecs::world ecs;

flecs::filter<> f = ecs.filter_builder()
.expr("(ChildOf,0)")
.build();

int32_t count = 0;
f.each_term([&](flecs::term &term) {
test_assert(term.id().is_pair());
count ++;
});

test_int(count, 1);
}
21 changes: 18 additions & 3 deletions test/cpp_api/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ void Query_test_no_defer_each(void);
void Query_test_no_defer_iter(void);
void Query_inspect_terms(void);
void Query_inspect_terms_w_each(void);
void Query_inspect_terms_w_expr(void);
void Query_comp_to_str(void);
void Query_pair_to_str(void);
void Query_oper_not_to_str(void);
Expand Down Expand Up @@ -777,6 +778,7 @@ void RuleBuilder_is_valid(void);
void RuleBuilder_unresolved_by_name(void);
void RuleBuilder_scope(void);
void RuleBuilder_iter_w_stage(void);
void RuleBuilder_inspect_terms_w_expr(void);

// Testsuite 'SystemBuilder'
void SystemBuilder_builder_assign_same_type(void);
Expand Down Expand Up @@ -857,6 +859,7 @@ void Filter_invalid_each_w_no_this(void);
void Filter_named_filter(void);
void Filter_named_scoped_filter(void);
void Filter_set_this_var(void);
void Filter_inspect_terms_w_expr(void);

// Testsuite 'ComponentLifecycle'
void ComponentLifecycle_ctor_on_add(void);
Expand Down Expand Up @@ -3257,6 +3260,10 @@ bake_test_case Query_testcases[] = {
"inspect_terms_w_each",
Query_inspect_terms_w_each
},
{
"inspect_terms_w_expr",
Query_inspect_terms_w_expr
},
{
"comp_to_str",
Query_comp_to_str
Expand Down Expand Up @@ -4289,6 +4296,10 @@ bake_test_case RuleBuilder_testcases[] = {
{
"iter_w_stage",
RuleBuilder_iter_w_stage
},
{
"inspect_terms_w_expr",
RuleBuilder_inspect_terms_w_expr
}
};

Expand Down Expand Up @@ -4594,6 +4605,10 @@ bake_test_case Filter_testcases[] = {
{
"set_this_var",
Filter_set_this_var
},
{
"inspect_terms_w_expr",
Filter_inspect_terms_w_expr
}
};

Expand Down Expand Up @@ -6277,7 +6292,7 @@ static bake_test_suite suites[] = {
"Query",
NULL,
NULL,
80,
81,
Query_testcases
},
{
Expand All @@ -6298,7 +6313,7 @@ static bake_test_suite suites[] = {
"RuleBuilder",
NULL,
NULL,
29,
30,
RuleBuilder_testcases
},
{
Expand All @@ -6319,7 +6334,7 @@ static bake_test_suite suites[] = {
"Filter",
NULL,
NULL,
22,
23,
Filter_testcases
},
{
Expand Down

0 comments on commit 49478e7

Please sign in to comment.