Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#1453 Fix leak when deferring a batched set for a non trivial large c… #1475

Merged
merged 1 commit into from
Dec 17, 2024
Merged

#1453 Fix leak when deferring a batched set for a non trivial large c… #1475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions distr/flecs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10647,15 +10647,17 @@ void flecs_cmd_batch_for_entity(
ecs_xtor_t dtor = ti->hooks.dtor;
if (dtor) {
dtor(cmd->is._1.value, 1, ti);
cmd->is._1.value = NULL;
}
} else {
ecs_os_memcpy(ptr.ptr, cmd->is._1.value, ti->size);
}

flecs_stack_free(cmd->is._1.value, cmd->is._1.size);
cmd->is._1.value = NULL;

if (cmd->kind == EcsCmdSet) {
/* A set operation is add + copy + modified. We just did
* the add the copy, so the only thing that's left is a
* the add and copy, so the only thing that's left is a
* modified command, which will call the OnSet
* observers. */
cmd->kind = EcsCmdModified;
Expand Down
6 changes: 4 additions & 2 deletions src/entity.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5023,15 +5023,17 @@ void flecs_cmd_batch_for_entity(
ecs_xtor_t dtor = ti->hooks.dtor;
if (dtor) {
dtor(cmd->is._1.value, 1, ti);
cmd->is._1.value = NULL;
}
} else {
ecs_os_memcpy(ptr.ptr, cmd->is._1.value, ti->size);
}

flecs_stack_free(cmd->is._1.value, cmd->is._1.size);
cmd->is._1.value = NULL;

if (cmd->kind == EcsCmdSet) {
/* A set operation is add + copy + modified. We just did
* the add the copy, so the only thing that's left is a
* the add and copy, so the only thing that's left is a
* modified command, which will call the OnSet
* observers. */
cmd->kind = EcsCmdModified;
Expand Down
6 changes: 6 additions & 0 deletions test/core/project.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2116,6 +2116,12 @@
"defer_remove",
"defer_remove_two",
"defer_set",
"defer_set_large",
"defer_set_large_non_trivial",
"defer_set_non_trivial",
"defer_batched_set_large",
"defer_batched_set_large_non_trivial",
"defer_batched_set_non_trivial",
"defer_delete",
"defer_twice",
"defer_twice_in_progress",
Expand Down
242 changes: 242 additions & 0 deletions test/core/src/Commands.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,248 @@ void Commands_defer_set(void) {
ecs_fini(world);
}

static
void dummy_xtor(void *ptr, ecs_size_t count, const ecs_type_info_t *ti) { }

static
void dummy_move(void *ptr, void *src, ecs_size_t count, const ecs_type_info_t *ti) {
ecs_os_memcpy(ptr, src, count * ti->size);
}

typedef struct Large {
int32_t largeArray[2048];
} Large;

void Commands_defer_set_large(void) {
ecs_world_t *world = ecs_mini();

ECS_COMPONENT(world, Large);

ecs_entity_t e = ecs_new(world);

ecs_defer_begin(world);

Large *l = ecs_ensure(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
l->largeArray[i] = i;
}
ecs_modified(world, e, Large);

test_assert(!ecs_has(world, e, Large));
ecs_defer_end(world);

test_assert(ecs_has(world, e, Large));

{
const Large *l = ecs_get(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
test_int(l->largeArray[i], i);
}
}

ecs_fini(world);
}

void Commands_defer_set_large_non_trivial(void) {
ecs_world_t *world = ecs_mini();

ECS_COMPONENT(world, Large);

ecs_set_hooks(world, Large, {
.ctor = dummy_xtor,
.dtor = dummy_xtor,
.move = dummy_move
});

ecs_entity_t e = ecs_new(world);

ecs_defer_begin(world);

Large *l = ecs_ensure(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
l->largeArray[i] = i;
}
ecs_modified(world, e, Large);

test_assert(!ecs_has(world, e, Large));
ecs_defer_end(world);
test_assert(ecs_has(world, e, Large));

{
const Large *l = ecs_get(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
test_int(l->largeArray[i], i);
}
}

ecs_fini(world);
}

void Commands_defer_set_non_trivial(void) {
ecs_world_t *world = ecs_mini();

ECS_COMPONENT(world, Velocity);

ecs_set_hooks(world, Velocity, {
.ctor = dummy_xtor,
.dtor = dummy_xtor,
.move = dummy_move
});

ecs_entity_t e = ecs_new(world);

ecs_defer_begin(world);

ecs_set(world, e, Velocity, {1, 2});

test_assert(!ecs_has(world, e, Velocity));
ecs_defer_end(world);

test_assert(ecs_has(world, e, Velocity));

{
const Velocity *v = ecs_get(world, e, Velocity);
test_assert(v != NULL);
test_int(v->x, 1);
test_int(v->y, 2);
}

ecs_fini(world);
}

void Commands_defer_batched_set_large(void) {
ecs_world_t *world = ecs_mini();

ECS_COMPONENT(world, Position);
ECS_COMPONENT(world, Large);

ecs_entity_t e = ecs_new(world);

ecs_defer_begin(world);

ecs_set(world, e, Position, {10, 20});
Large *l = ecs_ensure(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
l->largeArray[i] = i;
}
ecs_modified(world, e, Large);

test_assert(!ecs_has(world, e, Position));
test_assert(!ecs_has(world, e, Large));
ecs_defer_end(world);

test_assert(ecs_has(world, e, Position));
test_assert(ecs_has(world, e, Large));

{
const Position *p = ecs_get(world, e, Position);
test_assert(p != NULL);
test_int(p->x, 10);
test_int(p->y, 20);

const Large *l = ecs_get(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
test_int(l->largeArray[i], i);
}
}

ecs_fini(world);
}

void Commands_defer_batched_set_large_non_trivial(void) {
ecs_world_t *world = ecs_mini();

ECS_COMPONENT(world, Position);
ECS_COMPONENT(world, Large);

ecs_set_hooks(world, Large, {
.ctor = dummy_xtor,
.dtor = dummy_xtor,
.move = dummy_move
});

ecs_entity_t e = ecs_new(world);

ecs_defer_begin(world);

ecs_set(world, e, Position, {10, 20});
Large *l = ecs_ensure(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
l->largeArray[i] = i;
}
ecs_modified(world, e, Large);

test_assert(!ecs_has(world, e, Position));
test_assert(!ecs_has(world, e, Large));
ecs_defer_end(world);

test_assert(ecs_has(world, e, Position));
test_assert(ecs_has(world, e, Large));

{
const Position *p = ecs_get(world, e, Position);
test_assert(p != NULL);
test_int(p->x, 10);
test_int(p->y, 20);

const Large *l = ecs_get(world, e, Large);
test_assert(l != NULL);
for (int i = 0; i < 2048; i ++) {
test_int(l->largeArray[i], i);
}
}

ecs_fini(world);
}

void Commands_defer_batched_set_non_trivial(void) {
ecs_world_t *world = ecs_mini();

ECS_COMPONENT(world, Position);
ECS_COMPONENT(world, Velocity);

ecs_set_hooks(world, Velocity, {
.ctor = dummy_xtor,
.dtor = dummy_xtor,
.move = dummy_move
});

ecs_entity_t e = ecs_new(world);

ecs_defer_begin(world);

ecs_set(world, e, Position, {10, 20});
ecs_set(world, e, Velocity, {1, 2});

test_assert(!ecs_has(world, e, Position));
test_assert(!ecs_has(world, e, Velocity));
ecs_defer_end(world);

test_assert(ecs_has(world, e, Position));
test_assert(ecs_has(world, e, Velocity));

{
const Position *p = ecs_get(world, e, Position);
test_assert(p != NULL);
test_int(p->x, 10);
test_int(p->y, 20);

const Velocity *v = ecs_get(world, e, Velocity);
test_assert(v != NULL);
test_int(v->x, 1);
test_int(v->y, 2);
}

ecs_fini(world);
}

void Commands_defer_delete(void) {
ecs_world_t *world = ecs_mini();

Expand Down
32 changes: 31 additions & 1 deletion test/core/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2035,6 +2035,12 @@ void Commands_defer_add_two(void);
void Commands_defer_remove(void);
void Commands_defer_remove_two(void);
void Commands_defer_set(void);
void Commands_defer_set_large(void);
void Commands_defer_set_large_non_trivial(void);
void Commands_defer_set_non_trivial(void);
void Commands_defer_batched_set_large(void);
void Commands_defer_batched_set_large_non_trivial(void);
void Commands_defer_batched_set_non_trivial(void);
void Commands_defer_delete(void);
void Commands_defer_twice(void);
void Commands_defer_twice_in_progress(void);
Expand Down Expand Up @@ -10208,6 +10214,30 @@ bake_test_case Commands_testcases[] = {
"defer_set",
Commands_defer_set
},
{
"defer_set_large",
Commands_defer_set_large
},
{
"defer_set_large_non_trivial",
Commands_defer_set_large_non_trivial
},
{
"defer_set_non_trivial",
Commands_defer_set_non_trivial
},
{
"defer_batched_set_large",
Commands_defer_batched_set_large
},
{
"defer_batched_set_large_non_trivial",
Commands_defer_batched_set_large_non_trivial
},
{
"defer_batched_set_non_trivial",
Commands_defer_batched_set_non_trivial
},
{
"defer_delete",
Commands_defer_delete
Expand Down Expand Up @@ -11635,7 +11665,7 @@ static bake_test_suite suites[] = {
"Commands",
NULL,
NULL,
143,
149,
Commands_testcases
},
{
Expand Down
Loading