Skip to content

Commit

Permalink
#1450 Fix issue where batched commands for implicitly registered comp…
Browse files Browse the repository at this point in the history
…onent could crash
  • Loading branch information
SanderMertens authored Nov 25, 2024
1 parent e3219d7 commit e104de4
Show file tree
Hide file tree
Showing 11 changed files with 132 additions and 92 deletions.
55 changes: 44 additions & 11 deletions distr/flecs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2810,6 +2810,14 @@ ecs_allocator_t* flecs_stage_get_allocator(
ecs_stack_t* flecs_stage_get_stack_allocator(
ecs_world_t *world);

void flecs_commands_init(
ecs_stage_t *stage,
ecs_commands_t *cmd);

void flecs_commands_fini(
ecs_stage_t *stage,
ecs_commands_t *cmd);

#endif

/**
Expand Down Expand Up @@ -2897,6 +2905,36 @@ void flecs_delete_table(
void flecs_process_pending_tables(
const ecs_world_t *world);

/* Suspend/resume readonly state. To fully support implicit registration of
* components, it should be possible to register components while the world is
* in readonly mode. It is not uncommon that a component is used first from
* within a system, which are often ran while in readonly mode.
*
* Suspending readonly mode is only allowed when the world is not multithreaded.
* When a world is multithreaded, it is not safe to (even temporarily) leave
* readonly mode, so a multithreaded application should always explicitly
* register components in advance.
*
* These operations also suspend deferred mode.
*/
typedef struct ecs_suspend_readonly_state_t {
bool is_readonly;
bool is_deferred;
int32_t defer_count;
ecs_entity_t scope;
ecs_entity_t with;
ecs_commands_t cmd;
ecs_stage_t *stage;
} ecs_suspend_readonly_state_t;

ecs_world_t* flecs_suspend_readonly(
const ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

void flecs_resume_readonly(
ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

/* Convenience macro's for world allocator */
#define flecs_walloc(world, size)\
flecs_alloc(&world->allocator, size)
Expand Down Expand Up @@ -17152,7 +17190,6 @@ void flecs_stage_merge_post_frame(
ecs_vec_clear(&stage->post_frame_actions);
}

static
void flecs_commands_init(
ecs_stage_t *stage,
ecs_commands_t *cmd)
Expand All @@ -17163,13 +17200,13 @@ void flecs_commands_init(
&stage->allocators.cmd_entry_chunk, ecs_cmd_entry_t);
}

static
void flecs_commands_fini(
ecs_stage_t *stage,
ecs_commands_t *cmd)
{
/* Make sure stage has no unmerged data */
ecs_assert(ecs_vec_count(&stage->cmd->queue) == 0, ECS_INTERNAL_ERROR, NULL);
ecs_assert(ecs_vec_count(&stage->cmd->queue) == 0,
ECS_INTERNAL_ERROR, NULL);

flecs_stack_fini(&cmd->stack);
ecs_vec_fini_t(&stage->allocator, &cmd->queue, ecs_cmd_t);
Expand Down Expand Up @@ -18133,13 +18170,11 @@ ecs_world_t* flecs_suspend_readonly(

/* Hack around safety checks (this ought to look ugly) */
state->defer_count = stage->defer;
state->commands = stage->cmd->queue;
state->defer_stack = stage->cmd->stack;
flecs_stack_init(&stage->cmd->stack);
state->cmd = *stage->cmd;
flecs_commands_init(stage, stage->cmd);
state->scope = stage->scope;
state->with = stage->with;
stage->defer = 0;
ecs_vec_init_t(NULL, &stage->cmd->queue, ecs_cmd_t, 0);

return world;
}
Expand All @@ -18162,10 +18197,8 @@ void flecs_resume_readonly(
/* Restore readonly state / defer count */
ECS_BIT_COND(world->flags, EcsWorldReadonly, state->is_readonly);
stage->defer = state->defer_count;
ecs_vec_fini_t(&stage->allocator, &stage->cmd->queue, ecs_cmd_t);
stage->cmd->queue = state->commands;
flecs_stack_fini(&stage->cmd->stack);
stage->cmd->stack = state->defer_stack;
flecs_commands_fini(stage, stage->cmd);
*stage->cmd = state->cmd;
stage->scope = state->scope;
stage->with = state->with;
}
Expand Down
33 changes: 0 additions & 33 deletions distr/flecs.h
Original file line number Diff line number Diff line change
Expand Up @@ -3896,39 +3896,6 @@ FLECS_DBG_API
void flecs_dump_backtrace(
void *stream);

/* Suspend/resume readonly state. To fully support implicit registration of
* components, it should be possible to register components while the world is
* in readonly mode. It is not uncommon that a component is used first from
* within a system, which are often ran while in readonly mode.
*
* Suspending readonly mode is only allowed when the world is not multithreaded.
* When a world is multithreaded, it is not safe to (even temporarily) leave
* readonly mode, so a multithreaded application should always explicitly
* register components in advance.
*
* These operations also suspend deferred mode.
*/
typedef struct ecs_suspend_readonly_state_t {
bool is_readonly;
bool is_deferred;
int32_t defer_count;
ecs_entity_t scope;
ecs_entity_t with;
ecs_vec_t commands;
ecs_stack_t defer_stack;
ecs_stage_t *stage;
} ecs_suspend_readonly_state_t;

FLECS_API
ecs_world_t* flecs_suspend_readonly(
const ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

FLECS_API
void flecs_resume_readonly(
ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

FLECS_API
int32_t flecs_poly_claim_(
ecs_poly_t *poly);
Expand Down
4 changes: 2 additions & 2 deletions docs/FlecsScript.md
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ enum Color {
```

## Templates
Templates are parametrized scripts that can be used to create procedural assets. Templates can be created with the `template` keyword. Example:
Templates are parameterized scripts that can be used to create procedural assets. Templates can be created with the `template` keyword. Example:

```c
template Square {
Expand Down Expand Up @@ -640,7 +640,7 @@ Templates are commonly used in combination with the kind syntax:
Square my_entity
```

Templates can be parametrized with properties. Properties are variables that are exposed as component members. To create a property, use the `prop` keyword. Example:
Templates can be parameterized with properties. Properties are variables that are exposed as component members. To create a property, use the `prop` keyword. Example:

```c
template Square {
Expand Down
33 changes: 0 additions & 33 deletions include/flecs/private/api_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,39 +147,6 @@ FLECS_DBG_API
void flecs_dump_backtrace(
void *stream);

/* Suspend/resume readonly state. To fully support implicit registration of
* components, it should be possible to register components while the world is
* in readonly mode. It is not uncommon that a component is used first from
* within a system, which are often ran while in readonly mode.
*
* Suspending readonly mode is only allowed when the world is not multithreaded.
* When a world is multithreaded, it is not safe to (even temporarily) leave
* readonly mode, so a multithreaded application should always explicitly
* register components in advance.
*
* These operations also suspend deferred mode.
*/
typedef struct ecs_suspend_readonly_state_t {
bool is_readonly;
bool is_deferred;
int32_t defer_count;
ecs_entity_t scope;
ecs_entity_t with;
ecs_vec_t commands;
ecs_stack_t defer_stack;
ecs_stage_t *stage;
} ecs_suspend_readonly_state_t;

FLECS_API
ecs_world_t* flecs_suspend_readonly(
const ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

FLECS_API
void flecs_resume_readonly(
ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

FLECS_API
int32_t flecs_poly_claim_(
ecs_poly_t *poly);
Expand Down
5 changes: 2 additions & 3 deletions src/stage.c
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,6 @@ void flecs_stage_merge_post_frame(
ecs_vec_clear(&stage->post_frame_actions);
}

static
void flecs_commands_init(
ecs_stage_t *stage,
ecs_commands_t *cmd)
Expand All @@ -579,13 +578,13 @@ void flecs_commands_init(
&stage->allocators.cmd_entry_chunk, ecs_cmd_entry_t);
}

static
void flecs_commands_fini(
ecs_stage_t *stage,
ecs_commands_t *cmd)
{
/* Make sure stage has no unmerged data */
ecs_assert(ecs_vec_count(&stage->cmd->queue) == 0, ECS_INTERNAL_ERROR, NULL);
ecs_assert(ecs_vec_count(&stage->cmd->queue) == 0,
ECS_INTERNAL_ERROR, NULL);

flecs_stack_fini(&cmd->stack);
ecs_vec_fini_t(&stage->allocator, &cmd->queue, ecs_cmd_t);
Expand Down
8 changes: 8 additions & 0 deletions src/stage.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,12 @@ ecs_allocator_t* flecs_stage_get_allocator(
ecs_stack_t* flecs_stage_get_stack_allocator(
ecs_world_t *world);

void flecs_commands_init(
ecs_stage_t *stage,
ecs_commands_t *cmd);

void flecs_commands_fini(
ecs_stage_t *stage,
ecs_commands_t *cmd);

#endif
12 changes: 4 additions & 8 deletions src/world.c
Original file line number Diff line number Diff line change
Expand Up @@ -406,13 +406,11 @@ ecs_world_t* flecs_suspend_readonly(

/* Hack around safety checks (this ought to look ugly) */
state->defer_count = stage->defer;
state->commands = stage->cmd->queue;
state->defer_stack = stage->cmd->stack;
flecs_stack_init(&stage->cmd->stack);
state->cmd = *stage->cmd;
flecs_commands_init(stage, stage->cmd);
state->scope = stage->scope;
state->with = stage->with;
stage->defer = 0;
ecs_vec_init_t(NULL, &stage->cmd->queue, ecs_cmd_t, 0);

return world;
}
Expand All @@ -435,10 +433,8 @@ void flecs_resume_readonly(
/* Restore readonly state / defer count */
ECS_BIT_COND(world->flags, EcsWorldReadonly, state->is_readonly);
stage->defer = state->defer_count;
ecs_vec_fini_t(&stage->allocator, &stage->cmd->queue, ecs_cmd_t);
stage->cmd->queue = state->commands;
flecs_stack_fini(&stage->cmd->stack);
stage->cmd->stack = state->defer_stack;
flecs_commands_fini(stage, stage->cmd);
*stage->cmd = state->cmd;
stage->scope = state->scope;
stage->with = state->with;
}
Expand Down
30 changes: 30 additions & 0 deletions src/world.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,36 @@ void flecs_delete_table(
void flecs_process_pending_tables(
const ecs_world_t *world);

/* Suspend/resume readonly state. To fully support implicit registration of
* components, it should be possible to register components while the world is
* in readonly mode. It is not uncommon that a component is used first from
* within a system, which are often ran while in readonly mode.
*
* Suspending readonly mode is only allowed when the world is not multithreaded.
* When a world is multithreaded, it is not safe to (even temporarily) leave
* readonly mode, so a multithreaded application should always explicitly
* register components in advance.
*
* These operations also suspend deferred mode.
*/
typedef struct ecs_suspend_readonly_state_t {
bool is_readonly;
bool is_deferred;
int32_t defer_count;
ecs_entity_t scope;
ecs_entity_t with;
ecs_commands_t cmd;
ecs_stage_t *stage;
} ecs_suspend_readonly_state_t;

ecs_world_t* flecs_suspend_readonly(
const ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

void flecs_resume_readonly(
ecs_world_t *world,
ecs_suspend_readonly_state_t *state);

/* Convenience macro's for world allocator */
#define flecs_walloc(world, size)\
flecs_alloc(&world->allocator, size)
Expand Down
3 changes: 2 additions & 1 deletion test/core/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -2216,7 +2216,8 @@
"add_batched_set_with",
"defer_emplace_after_remove",
"batched_w_table_change_in_observer",
"redefine_named_in_threaded_app"
"redefine_named_in_threaded_app",
"batched_cmd_w_component_init"
]
}, {
"id": "SingleThreadStaging",
Expand Down
34 changes: 34 additions & 0 deletions test/core/src/Commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -4209,3 +4209,37 @@ void Commands_redefine_named_in_threaded_app(void) {

ecs_fini(world);
}

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

ECS_TAG(world, Foo);

ecs_entity_t parent = ecs_new(world);

ecs_defer_begin(world);

ecs_add(world, parent, Foo);

ecs_entity_t comp = ecs_component(world, {
.entity = ecs_entity(world, {
.parent = parent,
.name = "Bar"
}),
.type.size = 4,
.type.alignment = 4
});

test_assert(comp != 0);
test_str(ecs_get_name(world, comp), "Bar");
{
const EcsComponent *ptr = ecs_get(world, comp, EcsComponent);
test_assert(ptr != NULL);
test_int(ptr->size, 4);
test_int(ptr->alignment, 4);
}

ecs_defer_end(world);

ecs_fini(world);
}
7 changes: 6 additions & 1 deletion test/core/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -2136,6 +2136,7 @@ void Commands_add_batched_set_with(void);
void Commands_defer_emplace_after_remove(void);
void Commands_batched_w_table_change_in_observer(void);
void Commands_redefine_named_in_threaded_app(void);
void Commands_batched_cmd_w_component_init(void);

// Testsuite 'SingleThreadStaging'
void SingleThreadStaging_setup(void);
Expand Down Expand Up @@ -10576,6 +10577,10 @@ bake_test_case Commands_testcases[] = {
{
"redefine_named_in_threaded_app",
Commands_redefine_named_in_threaded_app
},
{
"batched_cmd_w_component_init",
Commands_batched_cmd_w_component_init
}
};

Expand Down Expand Up @@ -11460,7 +11465,7 @@ static bake_test_suite suites[] = {
"Commands",
NULL,
NULL,
141,
142,
Commands_testcases
},
{
Expand Down

0 comments on commit e104de4

Please sign in to comment.