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.

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

Fail CI on memory leaks #1008

Merged
merged 5 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 20 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,13 @@ jobs:
scan-build --status-bugs bake

analyze-sanitized-api:
needs: build-macos
runs-on: macOS-latest
needs: [ build-linux ]
runs-on: ${{ matrix.os }}
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest ]

steps:
- uses: actions/checkout@v3
Expand All @@ -432,11 +434,13 @@ jobs:
bake run test/api --cfg sanitize -- -j 8

analyze-sanitized-addons:
needs: build-macos
runs-on: macOS-latest
needs: [ build-linux ]
runs-on: ${{ matrix.os }}
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest ]

steps:
- uses: actions/checkout@v3
Expand All @@ -454,11 +458,13 @@ jobs:
bake run test/addons --cfg sanitize -- -j 8

analyze-sanitized-meta:
needs: build-macos
runs-on: macOS-latest
needs: [ build-linux ]
runs-on: ${{ matrix.os }}
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest ]

steps:
- uses: actions/checkout@v3
Expand All @@ -476,11 +482,13 @@ jobs:
bake run test/meta --cfg sanitize -- -j 8

analyze-sanitized-collections:
needs: build-macos
runs-on: macOS-latest
needs: [ build-linux ]
runs-on: ${{ matrix.os }}
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest ]

steps:
- uses: actions/checkout@v3
Expand All @@ -498,11 +506,13 @@ jobs:
bake run test/collections --cfg sanitize -- -j 8

analyze-sanitized-cpp_api:
needs: build-macos
runs-on: macOS-latest
needs: [ build-linux ]
runs-on: ${{ matrix.os }}
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest ]

steps:
- uses: actions/checkout@v3
Expand Down
49 changes: 17 additions & 32 deletions flecs.c
Original file line number Diff line number Diff line change
Expand Up @@ -16913,7 +16913,7 @@ void flecs_workers_progress(
void flecs_create_worker_threads(
ecs_world_t *world);

bool flecs_join_worker_threads(
void flecs_join_worker_threads(
ecs_world_t *world);

void flecs_signal_workers(
Expand Down Expand Up @@ -17093,8 +17093,8 @@ void flecs_signal_workers(
ecs_os_cond_broadcast(world->worker_cond);
ecs_os_mutex_unlock(world->sync_mutex);
}
bool flecs_join_worker_threads(

void flecs_join_worker_threads(
ecs_world_t *world)
{
ecs_poly_assert(world, ecs_world_t);
Expand All @@ -17110,12 +17110,11 @@ bool flecs_join_worker_threads(
threads_active = true;
break;
}
stage->thread = 0;
};

/* If no threads are active, just return */
if (!threads_active) {
return false;
return;
}

/* Make sure all threads are running, to ensure they catch the signal */
Expand All @@ -17128,7 +17127,6 @@ bool flecs_join_worker_threads(
/* Join all threads with main */
for (i = 1; i < count; i ++) {
if (ecs_using_task_threads(world)) {
/* Join using the override provided */
ecs_os_task_join(stages[i].thread);
} else {
ecs_os_thread_join(stages[i].thread);
Expand All @@ -17138,25 +17136,6 @@ bool flecs_join_worker_threads(

world->flags &= ~EcsWorldQuitWorkers;
ecs_assert(world->workers_running == 0, ECS_INTERNAL_ERROR, NULL);

return true;
}

/** Stop workers */
static
bool ecs_stop_threads(
ecs_world_t *world)
{
/* Join all existing worker threads */
if (flecs_join_worker_threads(world))
{
/* Deinitialize stages */
ecs_set_stage_count(world, 1);

return true;
}

return false;
}

/* -- Private functions -- */
Expand Down Expand Up @@ -17188,21 +17167,27 @@ void flecs_set_threads_internal(
? ecs_os_has_task_support()
: ecs_os_has_threading()),
ECS_MISSING_OS_API, NULL);

int32_t stage_count = ecs_get_stage_count(world);
bool worker_method_changed = (use_task_api != world->workers_use_task_api);
if (stage_count != threads || worker_method_changed) {

if ((stage_count != threads) || worker_method_changed) {
/* Stop existing threads */
if (stage_count > 1) {
if (ecs_stop_threads(world)) {
flecs_join_worker_threads(world);
ecs_set_stage_count(world, 1);

if (world->worker_cond) {
ecs_os_cond_free(world->worker_cond);
}
if (world->sync_cond) {
ecs_os_cond_free(world->sync_cond);
}
if (world->sync_mutex) {
ecs_os_mutex_free(world->sync_mutex);
}
}

/* Adopt the desired API for worker creation & join */
world->workers_use_task_api = use_task_api;

/* Start threads if number of threads > 1 */
Expand All @@ -17221,14 +17206,14 @@ void ecs_set_threads(
ecs_world_t *world,
int32_t threads)
{
flecs_set_threads_internal(world, threads, false /* use thread API*/);
flecs_set_threads_internal(world, threads, false /* use thread API */);
}

void ecs_set_task_threads(
ecs_world_t *world,
int32_t task_threads)
{
flecs_set_threads_internal(world, task_threads, true /* use task API*/);
flecs_set_threads_internal(world, task_threads, true /* use task API */);
}

bool ecs_using_task_threads(
Expand Down
4 changes: 2 additions & 2 deletions flecs.h
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,8 @@ extern "C" {
#define EcsQueryIsOrphaned (1u << 3u) /* Is subquery orphaned */
#define EcsQueryHasOutTerms (1u << 4u) /* Does query have out terms */
#define EcsQueryHasNonThisOutTerms (1u << 5u) /* Does query have non-this out terms */
#define EcsQueryHasMonitor (1u << 5u) /* Does query track changes */
#define EcsQueryTrivialIter (1u << 6u) /* Does the query require special features to iterate */
#define EcsQueryHasMonitor (1u << 6u) /* Does query track changes */
#define EcsQueryTrivialIter (1u << 7u) /* Does the query require special features to iterate */


////////////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 2 additions & 2 deletions include/flecs/private/api_flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ extern "C" {
#define EcsQueryIsOrphaned (1u << 3u) /* Is subquery orphaned */
#define EcsQueryHasOutTerms (1u << 4u) /* Does query have out terms */
#define EcsQueryHasNonThisOutTerms (1u << 5u) /* Does query have non-this out terms */
#define EcsQueryHasMonitor (1u << 5u) /* Does query track changes */
#define EcsQueryTrivialIter (1u << 6u) /* Does the query require special features to iterate */
#define EcsQueryHasMonitor (1u << 6u) /* Does query track changes */
#define EcsQueryTrivialIter (1u << 7u) /* Does the query require special features to iterate */


////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion src/addons/pipeline/pipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void flecs_workers_progress(
void flecs_create_worker_threads(
ecs_world_t *world);

bool flecs_join_worker_threads(
void flecs_join_worker_threads(
ecs_world_t *world);

void flecs_signal_workers(
Expand Down
47 changes: 16 additions & 31 deletions src/addons/pipeline/worker.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ void flecs_signal_workers(
ecs_os_cond_broadcast(world->worker_cond);
ecs_os_mutex_unlock(world->sync_mutex);
}
bool flecs_join_worker_threads(

void flecs_join_worker_threads(
ecs_world_t *world)
{
ecs_poly_assert(world, ecs_world_t);
Expand All @@ -194,12 +194,11 @@ bool flecs_join_worker_threads(
threads_active = true;
break;
}
stage->thread = 0;
};

/* If no threads are active, just return */
if (!threads_active) {
return false;
return;
}

/* Make sure all threads are running, to ensure they catch the signal */
Expand All @@ -212,7 +211,6 @@ bool flecs_join_worker_threads(
/* Join all threads with main */
for (i = 1; i < count; i ++) {
if (ecs_using_task_threads(world)) {
/* Join using the override provided */
ecs_os_task_join(stages[i].thread);
} else {
ecs_os_thread_join(stages[i].thread);
Expand All @@ -222,25 +220,6 @@ bool flecs_join_worker_threads(

world->flags &= ~EcsWorldQuitWorkers;
ecs_assert(world->workers_running == 0, ECS_INTERNAL_ERROR, NULL);

return true;
}

/** Stop workers */
static
bool ecs_stop_threads(
ecs_world_t *world)
{
/* Join all existing worker threads */
if (flecs_join_worker_threads(world))
{
/* Deinitialize stages */
ecs_set_stage_count(world, 1);

return true;
}

return false;
}

/* -- Private functions -- */
Expand Down Expand Up @@ -272,21 +251,27 @@ void flecs_set_threads_internal(
? ecs_os_has_task_support()
: ecs_os_has_threading()),
ECS_MISSING_OS_API, NULL);

int32_t stage_count = ecs_get_stage_count(world);
bool worker_method_changed = (use_task_api != world->workers_use_task_api);
if (stage_count != threads || worker_method_changed) {

if ((stage_count != threads) || worker_method_changed) {
/* Stop existing threads */
if (stage_count > 1) {
if (ecs_stop_threads(world)) {
flecs_join_worker_threads(world);
ecs_set_stage_count(world, 1);

if (world->worker_cond) {
ecs_os_cond_free(world->worker_cond);
}
if (world->sync_cond) {
ecs_os_cond_free(world->sync_cond);
}
if (world->sync_mutex) {
ecs_os_mutex_free(world->sync_mutex);
}
}

/* Adopt the desired API for worker creation & join */
world->workers_use_task_api = use_task_api;

/* Start threads if number of threads > 1 */
Expand All @@ -305,14 +290,14 @@ void ecs_set_threads(
ecs_world_t *world,
int32_t threads)
{
flecs_set_threads_internal(world, threads, false /* use thread API*/);
flecs_set_threads_internal(world, threads, false /* use thread API */);
}

void ecs_set_task_threads(
ecs_world_t *world,
int32_t task_threads)
{
flecs_set_threads_internal(world, task_threads, true /* use task API*/);
flecs_set_threads_internal(world, task_threads, true /* use task API */);
}

bool ecs_using_task_threads(
Expand Down
12 changes: 12 additions & 0 deletions test/addons/src/Metrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,8 @@ void Metrics_oneof_gauge_3_entities() {
}
test_bool(false, ecs_filter_next(&it));

ecs_filter_fini(f);

ecs_fini(world);
}

Expand Down Expand Up @@ -966,6 +968,8 @@ void Metrics_oneof_gauge_3_entities_1_existing() {
}
test_bool(false, ecs_filter_next(&it));

ecs_filter_fini(f);

ecs_fini(world);
}

Expand Down Expand Up @@ -1076,6 +1080,8 @@ void Metrics_oneof_gauge_w_remove() {
test_bool(false, ecs_filter_next(&it));
}

ecs_filter_fini(f);

ecs_fini(world);
}

Expand Down Expand Up @@ -1186,6 +1192,8 @@ void Metrics_oneof_gauge_w_clear() {
test_bool(false, ecs_filter_next(&it));
}

ecs_filter_fini(f);

ecs_fini(world);
}

Expand Down Expand Up @@ -1296,6 +1304,8 @@ void Metrics_oneof_gauge_w_delete() {
test_bool(false, ecs_filter_next(&it));
}

ecs_filter_fini(f);

ecs_fini(world);
}

Expand Down Expand Up @@ -1620,6 +1630,8 @@ void Metrics_oneof_counter() {
test_bool(false, ecs_filter_next(&it));
}

ecs_filter_fini(f);

ecs_fini(world);
}

Expand Down
Loading