fix(lib,scheduler): allocation constructors and allocators report OOM instead of building NULL-name records (bug-1882)
make_alloc() and make_host_alloc() assigned strdup() results unchecked, so under memory exhaustion they returned a record whose resname/reserve/ host was NULL; merge_job_alloc()'s same_alloc_slot() (bug-1863) then strcmp()'d it (ASAN SEGV at xcealloc.c:337). make_alloc() also passed a NULL resname straight to strdup(). - xcealloc.c: make_alloc()/make_host_alloc() reject a NULL name (EINVAL) and check every strdup(), freeing what they built and returning NULL (ENOMEM); on failure make_host_alloc()'s caller keeps `allocs`. dup_host_alloc() frees its partial copy on failure instead of leaking. - scheduler.c sched_make_allocs(): a record that cannot be built fails the whole array (NULL, ENOMEM) instead of leaving a NULL entry. - seqjob/parjob my_alloc(): check every constructor; on failure free what was built (parjob also rolls back the remaining claims) and return -ENOMEM with ctx->alloc NULL -- the caller moves ctx.alloc onto the job whatever the rc. Tests: new xcealloc_oom_test (8, --wrap=strdup) and sched_make_allocs_oom_test (real scheduler_srcs + --wrap=strdup); a new -ENOMEM test in the seqjob and parjob allocator suites. All RED before (SEGV / "Expected NULL" / "Expected -12 Was 1"), GREEN after. xce-build.sh all asan: 115/115 suites OK, Go tests OK. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c02d8e5cf6
commit
f9146864a3
GPG key ID:
80533536B0EB651C
10 changed files with 563 additions and 12 deletions
|
|
@ -241,3 +241,17 @@ libxce_test = executable('xcelib_test',
|
|||
install: true)
|
||||
|
||||
test('libxce test', libxce_test, timeout: 120)
|
||||
|
||||
# bug-1882 regression: `-Wl,--wrap=strdup` linker symbol interposition (same
|
||||
# idiom as bug-1843's sched_record_lifecycle_test `--wrap=realloc`) so the
|
||||
# tests can make a chosen strdup() inside make_alloc()/make_host_alloc()
|
||||
# fail. Its own binary, so xcelib_test's other tests never see the wrap.
|
||||
libxce_alloc_oom_test = executable('xcealloc_oom_test',
|
||||
['testsuite/test_xcealloc_oom.c'],
|
||||
include_directories: [incdir, build_inc],
|
||||
dependencies: libxce_test_deps,
|
||||
c_args: ['-DXCE_TESTING'],
|
||||
link_args: ['-Wl,--wrap=strdup'],
|
||||
install: false)
|
||||
|
||||
test('libxce alloc OOM test', libxce_alloc_oom_test, timeout: 60)
|
||||
|
|
|
|||
210
src/lib/testsuite/test_xcealloc_oom.c
Normal file
210
src/lib/testsuite/test_xcealloc_oom.c
Normal file
|
|
@ -0,0 +1,210 @@
|
|||
/*
|
||||
* bug-1882 regression: make_alloc()/make_host_alloc() must never return a
|
||||
* record whose resname/reserve/host string is NULL. Before the fix they
|
||||
* assigned strdup() results unchecked, so an allocation failure produced a
|
||||
* half-built record, and merge_job_alloc()'s same_alloc_slot() later
|
||||
* strcmp()'d the NULL resname (SEGV at xcealloc.c:337).
|
||||
*
|
||||
* `-Wl,--wrap=strdup` (lib/meson.build, same linker-interposition idiom as
|
||||
* bug-1843's `--wrap=realloc` in sched_record_lifecycle_test) lets a test
|
||||
* make the Nth strdup() call return NULL. libxce is a static library, so the
|
||||
* wrap reaches the calls inside xcealloc.c too. __wrap_strdup() passes
|
||||
* through to __real_strdup() unless a test arms it; this is a separate
|
||||
* binary so xcelib_test's other tests are untouched.
|
||||
*/
|
||||
#include <errno.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
#include "unity.h"
|
||||
#include "xce/types.h"
|
||||
#include "xcealloc.h"
|
||||
|
||||
#include "controller.capnp.h"
|
||||
|
||||
extern char* __real_strdup(const char* s);
|
||||
|
||||
static int g_strdup_fail_at = -1; /* -1 == disabled */
|
||||
static int g_strdup_calls = 0;
|
||||
|
||||
char* __wrap_strdup(const char* s)
|
||||
{
|
||||
if (g_strdup_fail_at >= 0) {
|
||||
int this_call = g_strdup_calls++;
|
||||
|
||||
if (this_call == g_strdup_fail_at) {
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
return __real_strdup(s);
|
||||
}
|
||||
|
||||
/* Make the strdup() call with 0-based index `n` (counted from now) fail. */
|
||||
static void fail_strdup_at(int n)
|
||||
{
|
||||
g_strdup_calls = 0;
|
||||
g_strdup_fail_at = n;
|
||||
}
|
||||
|
||||
void setUp(void)
|
||||
{
|
||||
g_strdup_fail_at = -1;
|
||||
g_strdup_calls = 0;
|
||||
}
|
||||
|
||||
void tearDown(void)
|
||||
{
|
||||
g_strdup_fail_at = -1;
|
||||
}
|
||||
|
||||
static void test_make_alloc_null_resname_returns_null(void)
|
||||
{
|
||||
xce_alloc_t* a;
|
||||
|
||||
errno = 0;
|
||||
a = make_alloc(NULL, (char*)"", 1, 1.0f, 0);
|
||||
|
||||
TEST_ASSERT_NULL(a);
|
||||
TEST_ASSERT_EQUAL_INT(EINVAL, errno);
|
||||
}
|
||||
|
||||
static void test_make_alloc_resname_strdup_failure_returns_null(void)
|
||||
{
|
||||
xce_alloc_t* a;
|
||||
|
||||
fail_strdup_at(0);
|
||||
errno = 0;
|
||||
a = make_alloc((char*)"mem", (char*)"", 1, 1.0f, 0);
|
||||
|
||||
TEST_ASSERT_NULL(a);
|
||||
TEST_ASSERT_EQUAL_INT(ENOMEM, errno);
|
||||
}
|
||||
|
||||
static void test_make_alloc_reserve_strdup_failure_returns_null(void)
|
||||
{
|
||||
xce_alloc_t* a;
|
||||
|
||||
fail_strdup_at(1);
|
||||
errno = 0;
|
||||
a = make_alloc((char*)"mem", (char*)"r1", 1, 1.0f, 0);
|
||||
|
||||
TEST_ASSERT_NULL(a);
|
||||
TEST_ASSERT_EQUAL_INT(ENOMEM, errno);
|
||||
}
|
||||
|
||||
static void test_make_alloc_default_reserve_strdup_failure_returns_null(void)
|
||||
{
|
||||
xce_alloc_t* a;
|
||||
|
||||
fail_strdup_at(1); /* the strdup("") for a NULL reserve */
|
||||
a = make_alloc((char*)"mem", NULL, 1, 1.0f, 0);
|
||||
|
||||
TEST_ASSERT_NULL(a);
|
||||
}
|
||||
|
||||
static void test_make_alloc_success_sets_every_string(void)
|
||||
{
|
||||
xce_alloc_t* a = make_alloc((char*)"mem", NULL, 1, 1.0f, 0);
|
||||
|
||||
TEST_ASSERT_NOT_NULL(a);
|
||||
TEST_ASSERT_EQUAL_STRING("mem", a->resname);
|
||||
TEST_ASSERT_EQUAL_STRING("", a->reserve);
|
||||
|
||||
free_ResAlloc_ptr(&a);
|
||||
}
|
||||
|
||||
static void test_make_host_alloc_null_host_returns_null(void)
|
||||
{
|
||||
xce_host_alloc_t* h;
|
||||
|
||||
errno = 0;
|
||||
h = make_host_alloc(NULL, 0, NULL);
|
||||
|
||||
TEST_ASSERT_NULL(h);
|
||||
TEST_ASSERT_EQUAL_INT(EINVAL, errno);
|
||||
}
|
||||
|
||||
/* On failure the caller keeps ownership of `allocs`: freeing it afterwards
|
||||
* must be neither a double free nor a leak (ASAN checks both). */
|
||||
static void test_make_host_alloc_strdup_failure_leaves_allocs_to_caller(void)
|
||||
{
|
||||
xce_alloc_t** allocs;
|
||||
xce_host_alloc_t* h;
|
||||
|
||||
allocs = make_allocs(1, make_alloc((char*)"mem", NULL, 1, 1.0f, 0));
|
||||
TEST_ASSERT_NOT_NULL(allocs);
|
||||
|
||||
fail_strdup_at(0);
|
||||
errno = 0;
|
||||
h = make_host_alloc((char*)"h0", 1, allocs);
|
||||
|
||||
TEST_ASSERT_NULL(h);
|
||||
TEST_ASSERT_EQUAL_INT(ENOMEM, errno);
|
||||
|
||||
free_ResAlloc_list(1, allocs);
|
||||
}
|
||||
|
||||
static xce_job_alloc_t* one_host_job(const char* host)
|
||||
{
|
||||
xce_alloc_t** allocs;
|
||||
xce_host_alloc_t** hallocs;
|
||||
|
||||
allocs = make_allocs(1, make_alloc((char*)"mem", NULL, 1, 1.0f, 0));
|
||||
hallocs = make_host_allocs(1, make_host_alloc((char*)host, 1, allocs));
|
||||
|
||||
return make_job_alloc(7, 1, hallocs);
|
||||
}
|
||||
|
||||
/* merge_job_alloc() deep-copies a new src host via dup_host_alloc(), which
|
||||
* builds it with make_alloc()/make_host_alloc(): a failed strdup() there must
|
||||
* surface as -ENOMEM, never as a host or resource with a NULL name left in
|
||||
* dst for the next merge to strcmp(). */
|
||||
static void test_merge_job_alloc_strdup_failure_reports_enomem(void)
|
||||
{
|
||||
int n;
|
||||
|
||||
/* strdup() calls when copying one new host with one resource:
|
||||
* 0 = resname, 1 = reserve, 2 = host. Fail each in turn. */
|
||||
for (n = 0; n < 3; n++) {
|
||||
xce_job_alloc_t* dst = one_host_job("h0");
|
||||
xce_job_alloc_t* src = one_host_job("h1");
|
||||
int rc;
|
||||
int i;
|
||||
|
||||
TEST_ASSERT_NOT_NULL(dst);
|
||||
TEST_ASSERT_NOT_NULL(src);
|
||||
|
||||
fail_strdup_at(n);
|
||||
rc = merge_job_alloc(dst, src);
|
||||
g_strdup_fail_at = -1;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(-ENOMEM, rc, "merge must report OOM");
|
||||
for (i = 0; i < dst->nhosts; i++) {
|
||||
TEST_ASSERT_NOT_NULL(dst->allocs[i]);
|
||||
TEST_ASSERT_NOT_NULL(dst->allocs[i]->host);
|
||||
}
|
||||
|
||||
free_JobAlloc_ptr(&dst);
|
||||
free_JobAlloc_ptr(&src);
|
||||
}
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
|
||||
printf("\n>> xcealloc OOM test (bug-1882)\n\n");
|
||||
|
||||
RUN_TEST(test_make_alloc_null_resname_returns_null);
|
||||
RUN_TEST(test_make_alloc_resname_strdup_failure_returns_null);
|
||||
RUN_TEST(test_make_alloc_reserve_strdup_failure_returns_null);
|
||||
RUN_TEST(test_make_alloc_default_reserve_strdup_failure_returns_null);
|
||||
RUN_TEST(test_make_alloc_success_sets_every_string);
|
||||
RUN_TEST(test_make_host_alloc_null_host_returns_null);
|
||||
RUN_TEST(test_make_host_alloc_strdup_failure_leaves_allocs_to_caller);
|
||||
RUN_TEST(test_merge_job_alloc_strdup_failure_reports_enomem);
|
||||
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
@ -8,21 +8,34 @@
|
|||
#include "xce/types.h"
|
||||
#include "xcealloc.h"
|
||||
|
||||
/* make_alloc() never returns a record with a NULL resname or reserve
|
||||
* (bug-1882): a NULL resname is rejected (errno EINVAL) and a failed
|
||||
* strdup() frees what was built and returns NULL (errno ENOMEM), because
|
||||
* merge_job_alloc()'s slot matching and every other consumer strcmp() these
|
||||
* strings. */
|
||||
xce_alloc_t* make_alloc(char* resname, char* reserve, uint64_t instid,
|
||||
float value, int duration)
|
||||
{
|
||||
xce_alloc_t* alloc = NULL;
|
||||
|
||||
if (resname == NULL) {
|
||||
errno = EINVAL;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
alloc = (xce_alloc_t*)calloc(1, sizeof(*alloc));
|
||||
if (alloc == NULL) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
alloc->resname = strdup(resname);
|
||||
if (reserve != NULL) {
|
||||
alloc->reserve = strdup(reserve);
|
||||
} else {
|
||||
alloc->reserve = strdup("");
|
||||
alloc->reserve = strdup((reserve != NULL) ? reserve : "");
|
||||
if ((alloc->resname == NULL) || (alloc->reserve == NULL)) {
|
||||
FREEUP(alloc->resname);
|
||||
FREEUP(alloc->reserve);
|
||||
FREEUP(alloc);
|
||||
errno = ENOMEM;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
alloc->instid = instid;
|
||||
|
|
@ -78,18 +91,32 @@ xce_alloc_t** make_allocs(int nallocs, ...)
|
|||
return allocs;
|
||||
}
|
||||
|
||||
/* make_host_alloc() takes ownership of `allocs` only on success; on failure
|
||||
* (NULL host: EINVAL, failed strdup(): ENOMEM) it returns NULL and the
|
||||
* caller still owns `allocs` (bug-1882: never a record with a NULL host). */
|
||||
xce_host_alloc_t* make_host_alloc(char* host, int nalloc, xce_alloc_t** allocs)
|
||||
{
|
||||
xce_host_alloc_t* halloc = NULL;
|
||||
|
||||
if (host == NULL) {
|
||||
errno = EINVAL;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
halloc = (xce_host_alloc_t*)calloc(1, sizeof(*halloc));
|
||||
if (halloc == NULL) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
halloc->host = strdup(host);
|
||||
if (halloc->host == NULL) {
|
||||
FREEUP(halloc);
|
||||
errno = ENOMEM;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
halloc->nallocs = nalloc;
|
||||
halloc->allocs = allocs;
|
||||
halloc->host = strdup(host);
|
||||
|
||||
return halloc;
|
||||
}
|
||||
|
|
@ -219,12 +246,16 @@ static xce_host_alloc_t* dup_host_alloc(const xce_host_alloc_t* src)
|
|||
for (i = 0; i < src->nallocs; i++) {
|
||||
allocs[i] = dup_alloc(src->allocs[i]);
|
||||
if (allocs[i] == NULL) {
|
||||
free_ResAlloc_list(src->nallocs, allocs);
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
halloc = make_host_alloc(src->host, src->nallocs, allocs);
|
||||
if (halloc == NULL) {
|
||||
free_ResAlloc_list(src->nallocs, allocs);
|
||||
}
|
||||
|
||||
return halloc;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -690,20 +690,41 @@ static int my_alloc(xce_sched_plugin_t* plugin, xce_sched_ctx_t* ctx)
|
|||
return -ENOMEM;
|
||||
}
|
||||
|
||||
/* bug-1882: any constructor failing (OOM) is -ENOMEM with
|
||||
* ctx->alloc left NULL (the caller moves ctx->alloc onto the
|
||||
* job whatever the rc): free the host records built so far,
|
||||
* roll back the claims not yet turned into one. */
|
||||
for (i = 0; i < nclaimed; i++) {
|
||||
int nres = claimed[i].host_req.count;
|
||||
xce_alloc_t** allocs
|
||||
= sched_make_allocs(sched, "", claimed[i].host_req.count,
|
||||
= sched_make_allocs(sched, "", nres,
|
||||
(int*)claimed[i].host_req.idxs.data,
|
||||
(float*)claimed[i].host_req.vals.data,
|
||||
/* FIXME: no duration now */ NULL);
|
||||
|
||||
hallocs[i] = make_host_alloc(claimed[i].w->name,
|
||||
claimed[i].host_req.count, allocs);
|
||||
if ((allocs != NULL) || (nres <= 0)) {
|
||||
hallocs[i]
|
||||
= make_host_alloc(claimed[i].w->name, nres, allocs);
|
||||
if (hallocs[i] == NULL) {
|
||||
free_ResAlloc_list(nres, allocs);
|
||||
}
|
||||
}
|
||||
if (hallocs[i] == NULL) {
|
||||
parjob_rollback_claims_(ctx, claimed + i, nclaimed - i);
|
||||
free_HostAlloc_list(nclaimed, hallocs);
|
||||
plan_iter->fini(plan_iter);
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
xvec_idval_fini(&(claimed[i].host_req));
|
||||
}
|
||||
|
||||
ctx->alloc = make_job_alloc(0, nclaimed, hallocs);
|
||||
if (ctx->alloc == NULL) {
|
||||
free_HostAlloc_list(nclaimed, hallocs);
|
||||
plan_iter->fini(plan_iter);
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
if (ctx->alloc_req.shared.count > 0) {
|
||||
ctx->alloc->allocs_shared
|
||||
|
|
@ -711,6 +732,11 @@ static int my_alloc(xce_sched_plugin_t* plugin, xce_sched_ctx_t* ctx)
|
|||
ctx->alloc_req.shared.idxs.data,
|
||||
ctx->alloc_req.shared.vals.data,
|
||||
/* FIXME: no duration now */ NULL);
|
||||
if (ctx->alloc->allocs_shared == NULL) {
|
||||
free_JobAlloc_ptr(&(ctx->alloc));
|
||||
plan_iter->fini(plan_iter);
|
||||
return -ENOMEM;
|
||||
}
|
||||
ctx->alloc->nres_shared = ctx->alloc_req.shared.count;
|
||||
} else {
|
||||
ctx->alloc->allocs_shared = NULL;
|
||||
|
|
|
|||
|
|
@ -362,14 +362,35 @@ static int my_alloc(xce_sched_plugin_t* plugin, xce_sched_ctx_t* ctx)
|
|||
xce_host_alloc_t* halloc = NULL;
|
||||
xce_alloc_t** allocs = NULL;
|
||||
|
||||
/* bug-1882: any constructor failing (OOM) is -ENOMEM with
|
||||
* ctx->alloc left NULL -- the caller moves ctx->alloc onto the job
|
||||
* whatever the rc, so a half-built allocation must never stay
|
||||
* there. The debits this attempt made live in ctx's scratch and
|
||||
* are dropped by sched_ctx_fini(). */
|
||||
allocs = sched_make_allocs(
|
||||
sched, "", ctx->alloc_req.host.count, ctx->alloc_req.host.idxs.data,
|
||||
ctx->alloc_req.host.vals.data, /* FIXME: no duration now */ NULL);
|
||||
if ((allocs == NULL) && (ctx->alloc_req.host.count > 0)) {
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
halloc = make_host_alloc(w->name, ctx->alloc_req.host.count, allocs);
|
||||
if (halloc == NULL) {
|
||||
free_ResAlloc_list(ctx->alloc_req.host.count, allocs);
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
halloc = make_host_alloc(w->name, ctx->alloc_req.host.count, allocs);
|
||||
hallocs = make_host_allocs(1, halloc);
|
||||
if (hallocs == NULL) {
|
||||
free_HostAlloc_ptr(&halloc);
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
ctx->alloc = make_job_alloc(0, 1, hallocs);
|
||||
if (ctx->alloc == NULL) {
|
||||
free_HostAlloc_list(1, hallocs);
|
||||
return -ENOMEM;
|
||||
}
|
||||
|
||||
if (ctx->alloc_req.shared.count > 0) {
|
||||
ctx->alloc->allocs_shared
|
||||
|
|
@ -377,6 +398,10 @@ static int my_alloc(xce_sched_plugin_t* plugin, xce_sched_ctx_t* ctx)
|
|||
ctx->alloc_req.shared.idxs.data,
|
||||
ctx->alloc_req.shared.vals.data,
|
||||
/*FIXME: no duration now*/ NULL);
|
||||
if (ctx->alloc->allocs_shared == NULL) {
|
||||
free_JobAlloc_ptr(&(ctx->alloc));
|
||||
return -ENOMEM;
|
||||
}
|
||||
ctx->alloc->nres_shared = ctx->alloc_req.shared.count;
|
||||
} else {
|
||||
ctx->alloc->allocs_shared = NULL;
|
||||
|
|
|
|||
|
|
@ -1891,6 +1891,14 @@ xce_alloc_t** sched_make_allocs(xce_scheduler_t* sched, char* reserve, int nres,
|
|||
|
||||
allocs[i]
|
||||
= make_alloc(r->name, reserve, sched_get_id(sched), vals[i], d);
|
||||
if (allocs[i] == NULL) {
|
||||
/* bug-1882: a record that cannot be built fails the whole
|
||||
* array -- a NULL entry would reach the allocator plugins as a
|
||||
* host allocation silently missing this resource. */
|
||||
free_ResAlloc_list(nres, allocs);
|
||||
errno = ENOMEM;
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
return allocs;
|
||||
|
|
|
|||
|
|
@ -1917,6 +1917,29 @@ test('scheduler repl resource formatting test',
|
|||
test('scheduler connect pipeline test',
|
||||
sched_scheduler_connect_test, suite: 'scheduler',
|
||||
env: ['ASAN_OPTIONS=detect_leaks=0'])
|
||||
|
||||
# bug-1882 regression: sched_make_allocs() must fail the whole array when
|
||||
# make_alloc() cannot build a record. Real scheduler_srcs (as
|
||||
# sched_scheduler_connect_test) plus `-Wl,--wrap=strdup` to make a chosen
|
||||
# strdup() inside make_alloc() fail.
|
||||
sched_make_allocs_oom_test_srcs = ['test_sched_make_allocs_oom.c']
|
||||
foreach f : scheduler_srcs
|
||||
if f != 'src/main.c'
|
||||
sched_make_allocs_oom_test_srcs += ['../' + f]
|
||||
endif
|
||||
endforeach
|
||||
|
||||
sched_make_allocs_oom_test = executable(
|
||||
'sched_make_allocs_oom_test',
|
||||
sched_make_allocs_oom_test_srcs,
|
||||
include_directories:[incdir],
|
||||
dependencies: [scheduler_deps, unity_dep],
|
||||
link_args: ['-Wl,--wrap=strdup'],
|
||||
install: false)
|
||||
|
||||
test('scheduler make_allocs OOM test',
|
||||
sched_make_allocs_oom_test, suite: 'scheduler',
|
||||
env: ['ASAN_OPTIONS=detect_leaks=0'])
|
||||
test('scheduler decision publish retry test',
|
||||
sched_scheduler_decision_publish_retry_test, suite: 'scheduler',
|
||||
env: ['ASAN_OPTIONS=detect_leaks=0'])
|
||||
|
|
|
|||
|
|
@ -182,6 +182,11 @@ int plugin_job_rusage_update(void* s, xce_sched_job_t* j, xce_job_rusages_t* r)
|
|||
* assert_uncommitted_host_debit() (real ctx->uncomitted state, untouched
|
||||
* by this stub) rather than these fake xce_alloc_t's fields, exactly like
|
||||
* the seqjob equivalence tests already do. */
|
||||
/* bug-1882: arm to make the stub's Nth call (0-based) fail like the real
|
||||
* sched_make_allocs() does when make_alloc() cannot build a record. */
|
||||
static int g_make_allocs_fail_at = -1; /* -1 == disabled */
|
||||
static int g_make_allocs_calls = 0;
|
||||
|
||||
xce_alloc_t** sched_make_allocs(xce_scheduler_t* sched, char* reserve, int nres,
|
||||
int* idxs, float* vals, int* duration)
|
||||
{
|
||||
|
|
@ -194,6 +199,12 @@ xce_alloc_t** sched_make_allocs(xce_scheduler_t* sched, char* reserve, int nres,
|
|||
(void)vals;
|
||||
(void)duration;
|
||||
|
||||
if ((g_make_allocs_fail_at >= 0)
|
||||
&& (g_make_allocs_calls++ == g_make_allocs_fail_at)) {
|
||||
errno = ENOMEM;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (nres <= 0) {
|
||||
return NULL;
|
||||
}
|
||||
|
|
@ -730,6 +741,53 @@ static void test_multi_bucket_plan_succeeds_through_real_my_alloc(void)
|
|||
sched_ctx_fini(&ctx);
|
||||
}
|
||||
|
||||
/*
|
||||
* bug-1882: a two-host plan whose allocation records cannot be built --
|
||||
* first for host 0, then for host 1 after host 0's record already exists --
|
||||
* must report -ENOMEM with ctx->alloc left NULL (the caller moves ctx->alloc
|
||||
* onto the job whatever the rc). ASAN catches a double free or use after
|
||||
* free on the cleanup path.
|
||||
*/
|
||||
static void test_my_alloc_reports_enomem_when_allocs_cannot_be_built(void)
|
||||
{
|
||||
int fail_at;
|
||||
|
||||
for (fail_at = 0; fail_at < 2; fail_at++) {
|
||||
xce_scheduler_t sched;
|
||||
xce_resource_t* slots_res;
|
||||
xce_sched_obj_t obj;
|
||||
xce_primary_schedobj_t primary_obj;
|
||||
xce_primary_resreq_t primary;
|
||||
xce_sched_ctx_t ctx;
|
||||
int rc;
|
||||
|
||||
memset(&sched, 0, sizeof(sched));
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sched.core)));
|
||||
|
||||
slots_res = register_slots_resource(sched.core.resmgr);
|
||||
TEST_ASSERT_NOT_NULL(slots_res);
|
||||
add_worker_with_slots(&(sched.core), "host-a", slots_res->idx, 2.0f);
|
||||
add_worker_with_slots(&(sched.core), "host-b", slots_res->idx, 1.0f);
|
||||
|
||||
memset(&primary, 0, sizeof(primary));
|
||||
primary.minTasks = 1;
|
||||
primary.maxTasks = 3;
|
||||
primary.select = NULL;
|
||||
|
||||
init_primary_obj(&obj, &primary_obj, &primary);
|
||||
|
||||
g_make_allocs_calls = 0;
|
||||
g_make_allocs_fail_at = fail_at;
|
||||
rc = drive_my_alloc(&sched, &obj, &primary, &ctx);
|
||||
g_make_allocs_fail_at = -1;
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(-ENOMEM, rc);
|
||||
TEST_ASSERT_NULL(ctx.alloc);
|
||||
|
||||
sched_ctx_fini(&ctx);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Req 4 AC 2: the exact descending-vs-ascending worked example from
|
||||
* requirements.md's Composition Algorithm. Two hosts, capacities 1 and 2;
|
||||
|
|
@ -1157,7 +1215,11 @@ static void test_shared_resource_debited_once_regardless_of_host_count(void)
|
|||
sched_ctx_fini(&ctx);
|
||||
}
|
||||
|
||||
void setUp(void) { return; }
|
||||
void setUp(void)
|
||||
{
|
||||
g_make_allocs_fail_at = -1;
|
||||
g_make_allocs_calls = 0;
|
||||
}
|
||||
|
||||
void tearDown(void) { return; }
|
||||
|
||||
|
|
@ -1171,6 +1233,7 @@ int main(void)
|
|||
RUN_TEST(test_init_rejects_invalid_input);
|
||||
RUN_TEST(test_next_has_a_real_cancellation_point);
|
||||
RUN_TEST(test_multi_bucket_plan_succeeds_through_real_my_alloc);
|
||||
RUN_TEST(test_my_alloc_reports_enomem_when_allocs_cannot_be_built);
|
||||
RUN_TEST(test_descending_bucket_order_succeeds_where_ascending_fails);
|
||||
RUN_TEST(
|
||||
test_mid_plan_bucket_failure_rolls_back_before_later_plan_succeeds);
|
||||
|
|
|
|||
95
src/scheduler/testsuite/test_sched_make_allocs_oom.c
Normal file
95
src/scheduler/testsuite/test_sched_make_allocs_oom.c
Normal file
|
|
@ -0,0 +1,95 @@
|
|||
/*
|
||||
* bug-1882 regression, scheduler side: sched_make_allocs() (scheduler.c)
|
||||
* must return NULL when make_alloc() cannot build one of the records,
|
||||
* instead of an array with a NULL entry that the allocator plugins would
|
||||
* hand on as a host allocation silently missing a resource.
|
||||
*
|
||||
* Links the real scheduler_srcs (same technique as
|
||||
* sched_scheduler_connect_test) plus `-Wl,--wrap=strdup` (bug-1843's
|
||||
* `--wrap=realloc` idiom), so a chosen strdup() inside make_alloc() fails.
|
||||
*/
|
||||
#include <errno.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#include "resobj.h"
|
||||
#include "scheduler.h"
|
||||
#include "xcealloc.h"
|
||||
|
||||
extern char* __real_strdup(const char* s);
|
||||
|
||||
static int g_strdup_fail_at = -1; /* -1 == disabled */
|
||||
static int g_strdup_calls = 0;
|
||||
|
||||
char* __wrap_strdup(const char* s)
|
||||
{
|
||||
if (g_strdup_fail_at >= 0) {
|
||||
int this_call = g_strdup_calls++;
|
||||
|
||||
if (this_call == g_strdup_fail_at) {
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
return __real_strdup(s);
|
||||
}
|
||||
|
||||
void setUp(void)
|
||||
{
|
||||
g_strdup_fail_at = -1;
|
||||
g_strdup_calls = 0;
|
||||
}
|
||||
|
||||
void tearDown(void)
|
||||
{
|
||||
g_strdup_fail_at = -1;
|
||||
}
|
||||
|
||||
static void test_sched_make_allocs_returns_null_when_a_record_fails(void)
|
||||
{
|
||||
xce_scheduler_t sched;
|
||||
xce_resource_t res;
|
||||
xce_resource_t* slots;
|
||||
xce_alloc_t** allocs;
|
||||
int idxs[2];
|
||||
float vals[2] = { 1.0f, 2.0f };
|
||||
|
||||
memset(&sched, 0, sizeof(sched));
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sched.core)));
|
||||
|
||||
memset(&res, 0, sizeof(res));
|
||||
res.name = strdup("slots");
|
||||
slots = makeResource(sched.core.resmgr, &res);
|
||||
TEST_ASSERT_NOT_NULL(slots);
|
||||
idxs[0] = slots->idx;
|
||||
idxs[1] = slots->idx;
|
||||
|
||||
/* Control: both records built. */
|
||||
allocs = sched_make_allocs(&sched, (char*)"", 2, idxs, vals, NULL);
|
||||
TEST_ASSERT_NOT_NULL(allocs);
|
||||
TEST_ASSERT_NOT_NULL(allocs[0]);
|
||||
TEST_ASSERT_NOT_NULL(allocs[1]);
|
||||
free_ResAlloc_list(2, allocs);
|
||||
|
||||
/* strdup() calls: record 0 resname = 0, reserve = 1; record 1
|
||||
* resname = 2. Fail record 1's resname. */
|
||||
g_strdup_calls = 0;
|
||||
g_strdup_fail_at = 2;
|
||||
errno = 0;
|
||||
allocs = sched_make_allocs(&sched, (char*)"", 2, idxs, vals, NULL);
|
||||
g_strdup_fail_at = -1;
|
||||
|
||||
TEST_ASSERT_NULL_MESSAGE(allocs,
|
||||
"a record that cannot be built must fail the "
|
||||
"whole array, not leave a NULL entry");
|
||||
TEST_ASSERT_EQUAL_INT(ENOMEM, errno);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
RUN_TEST(test_sched_make_allocs_returns_null_when_a_record_fails);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
|
@ -134,6 +134,10 @@ int plugin_job_rusage_update(void* s, xce_sched_job_t* j, xce_job_rusages_t* r)
|
|||
* how many resource entries it carries, not the internal xce_alloc_t
|
||||
* field contents make_alloc() (xcealloc.c, unmodified, real) would fill
|
||||
* in for a live cluster. */
|
||||
/* bug-1882: set to make the stub fail like the real sched_make_allocs()
|
||||
* does when make_alloc() cannot build a record. */
|
||||
static int g_make_allocs_fail = 0;
|
||||
|
||||
xce_alloc_t** sched_make_allocs(xce_scheduler_t* sched, char* reserve,
|
||||
int nres, int* idxs, float* vals,
|
||||
int* duration)
|
||||
|
|
@ -147,6 +151,11 @@ xce_alloc_t** sched_make_allocs(xce_scheduler_t* sched, char* reserve,
|
|||
(void)vals;
|
||||
(void)duration;
|
||||
|
||||
if (g_make_allocs_fail) {
|
||||
errno = ENOMEM;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (nres <= 0) {
|
||||
return NULL;
|
||||
}
|
||||
|
|
@ -163,8 +172,8 @@ xce_alloc_t** sched_make_allocs(xce_scheduler_t* sched, char* reserve,
|
|||
return allocs;
|
||||
}
|
||||
|
||||
void setUp(void) { return; }
|
||||
void tearDown(void) { return; }
|
||||
void setUp(void) { g_make_allocs_fail = 0; }
|
||||
void tearDown(void) { g_make_allocs_fail = 0; }
|
||||
|
||||
/* ---------------------------------------------------------------------- */
|
||||
/* Fixture helpers */
|
||||
|
|
@ -379,9 +388,56 @@ test_seqjob_prepare_allocation_guards_against_maxtasks_gt_1_fanout(void)
|
|||
sched_ctx_fini(&ctx);
|
||||
}
|
||||
|
||||
/*
|
||||
* bug-1882: when the allocation records cannot be built (sched_make_allocs()
|
||||
* returns NULL), my_alloc() must report -ENOMEM and leave ctx->alloc NULL
|
||||
* -- sched_make_primary_allocation() moves ctx.alloc onto the job whatever
|
||||
* the rc, so a half-built allocation there would be dispatched.
|
||||
*/
|
||||
static void test_my_alloc_reports_enomem_when_allocs_cannot_be_built(void)
|
||||
{
|
||||
xce_scheduler_t sched;
|
||||
xce_resource_t* slots_res;
|
||||
xce_sched_obj_t obj;
|
||||
xce_primary_schedobj_t primary_obj;
|
||||
xce_primary_resreq_t primary;
|
||||
xce_sched_ctx_t ctx;
|
||||
xce_sched_plugin_t plugin;
|
||||
int rc;
|
||||
|
||||
memset(&sched, 0, sizeof(sched));
|
||||
TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&(sched.core)));
|
||||
|
||||
slots_res = register_slots_resource(sched.core.resmgr);
|
||||
TEST_ASSERT_NOT_NULL(slots_res);
|
||||
add_worker_with_slots(&(sched.core), WORKER_NAME, slots_res->idx, 4.0f);
|
||||
|
||||
memset(&primary, 0, sizeof(primary));
|
||||
primary.minTasks = 1;
|
||||
primary.maxTasks = 1;
|
||||
primary.select = NULL;
|
||||
|
||||
init_primary_obj(&obj, &primary_obj, &primary);
|
||||
TEST_ASSERT_EQUAL_INT(0, sched_ctx_init(&ctx, &(sched.core), &obj));
|
||||
sched_ctx_set_primary(&ctx, &primary);
|
||||
|
||||
memset(&plugin, 0, sizeof(plugin));
|
||||
plugin.sched = &sched;
|
||||
plugin.core = &(sched.core);
|
||||
|
||||
g_make_allocs_fail = 1;
|
||||
rc = my_alloc(&plugin, &ctx);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(-ENOMEM, rc);
|
||||
TEST_ASSERT_NULL(ctx.alloc);
|
||||
|
||||
sched_ctx_fini(&ctx);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
UNITY_BEGIN();
|
||||
RUN_TEST(test_my_alloc_reports_enomem_when_allocs_cannot_be_built);
|
||||
RUN_TEST(
|
||||
test_my_alloc_allocates_single_matching_host_through_shared_iterators);
|
||||
RUN_TEST(test_seqjob_prepare_allocation_guards_against_maxtasks_gt_1_fanout);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue