limit_get_configured_limits()/limit_get_usage_instances() cross the limit plugin's private-state boundary (plugin->data, previously unreachable outside limit.c) via key_find on the name-keyed plugin registry, returning NULL when the plugin is absent/uninitialized and a raw iterable kvmgr handle otherwise. Pure reads only -- no allocation, mutation, or hook triggering. Foundation for the upcoming `info limit` REPL command (task 1 of 5). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
432 lines
16 KiB
C
432 lines
16 KiB
C
/*
|
|
* test_limit_plugin.c - white-box regression test for bug-1685: a live
|
|
* SIGABRT (glibc malloc_printerr, "double free or corruption"-class heap
|
|
* corruption abort) in limit_update_config() (../src/plugins/limit.c)
|
|
* whenever a scheduler config update contains two xce_limit_config_t
|
|
* entries with the SAME limit name.
|
|
*
|
|
* Root cause: privdata->limits is a kvmgr created with
|
|
* .datakiller = kill_limit (limit_priv_data_init(), limit.c). key_add() on
|
|
* that kvmgr dispatches to xkvmgr_add() (../../lib/kvmgr.c), which -- when
|
|
* the SECOND, kvtab-level key_add() fails (e.g. -EEXIST for a duplicate
|
|
* name, after the value has already been vstore()'d) -- itself rolls back
|
|
* via vclear(kvm->data, idx). vclear() dispatches to xzarray_clear()
|
|
* (../../lib/zarray.c), which fires the configured killer (kill_limit) on
|
|
* the just-stored value exactly once as part of that rollback. Before this
|
|
* fix, limit_update_config()'s own error path called kill_limit(limit) a
|
|
* SECOND time on the same already-freed pointer -- a double free that
|
|
* glibc's malloc_printerr aborts on. The fix removes that second,
|
|
* redundant kill_limit() call from limit_update_config()'s error path (see
|
|
* that function's own comment for the full mechanism and the accepted,
|
|
* documented residual gap for the separate vstore()-failure sub-case).
|
|
*
|
|
* RED (pre-fix): running test_duplicate_limit_name_no_double_free()
|
|
* against a limit_update_config() that still calls kill_limit(limit) in
|
|
* its error path crashes with a real SIGABRT ("double free or corruption")
|
|
* on the second (duplicate-name) config entry -- not a clean
|
|
* TEST_ASSERT failure, an actual process abort, matching bug-1685's
|
|
* captured live crash signature (malloc_printerr <- kill_limit <-
|
|
* limit_update_config, rc=-17/-EEXIST at the crash frame).
|
|
* GREEN (post-fix): limit_update_config() returns a negative rc (-EEXIST)
|
|
* on the duplicate-name collision, with no crash and no double free.
|
|
*
|
|
* #include's the real ../src/plugins/limit.c (white-box, same convention
|
|
* as test_plugin_picker_registration.c/test_handlers.c/
|
|
* test_job_array_expand.c) so this test exercises the real
|
|
* limit_update_config()/limit_priv_data_init()/limit_init() -- not a
|
|
* re-implementation of any of them. All three are `static`; limit_init()/
|
|
* limit_priv_data_init() are otherwise unreachable outside this TU, and
|
|
* limit_update_config() is reachable in production only via
|
|
* plugin_config_update()'s hooks.config_hook fan-out (plugin.c) -- calling
|
|
* it directly here is the same "drive the static entry point directly"
|
|
* convention test_job_update.c/test_handlers.c use for their own static
|
|
* targets.
|
|
*
|
|
* Stubs: limit.c's limit_job_election()/limit_prepare_allocation()
|
|
* (neither exercised by this test -- only limit_update_config() is)
|
|
* reference reset_sched_obj() (../src/schedobj.c) and get_resvals()/
|
|
* put_resvals() (../src/resource.c), none of which is part of this
|
|
* white-box TU. Stubbed as no-ops purely to satisfy the linker -- same
|
|
* pattern test_plugin_picker_registration.c uses for its
|
|
* sched_fill_limit_requests() stub.
|
|
*/
|
|
#include <errno.h>
|
|
#include <stdlib.h>
|
|
#include <string.h>
|
|
|
|
#include <unity/unity.h>
|
|
|
|
#include "group.h"
|
|
#include "kvmgr.h"
|
|
#include "scheduler.h"
|
|
#include "xce/types.h"
|
|
#include "xcelimit.h"
|
|
|
|
void reset_sched_obj(xce_sched_obj_t* obj)
|
|
{
|
|
(void)obj;
|
|
}
|
|
|
|
int get_resvals(zarray_t* array, int nres, int* idxs, float* vals)
|
|
{
|
|
(void)array;
|
|
(void)nres;
|
|
(void)idxs;
|
|
(void)vals;
|
|
return 0;
|
|
}
|
|
|
|
int put_resvals(zarray_t* array, int nres, int* idxs, float* vals)
|
|
{
|
|
(void)array;
|
|
(void)nres;
|
|
(void)idxs;
|
|
(void)vals;
|
|
return 0;
|
|
}
|
|
|
|
/* register_limit() (limit.c, the plugin's public entry point) calls
|
|
* plugin_register() (../src/plugin.c, not part of this TU); this test
|
|
* never calls register_limit() itself (it drives limit_init()/
|
|
* limit_update_config() directly), so a no-op stub is sufficient purely
|
|
* to satisfy the linker -- same rationale as the other stubs above. */
|
|
int plugin_register(void* sched, void* plugin)
|
|
{
|
|
(void)sched;
|
|
(void)plugin;
|
|
return 0;
|
|
}
|
|
|
|
#include "../src/plugins/limit.c"
|
|
|
|
void setUp(void) { return; }
|
|
void tearDown(void) { return; }
|
|
|
|
/* ---------------------------------------------------------------------- */
|
|
/* Fixture: a real xce_scheduler_t with real group managers for
|
|
* core.ugmgr/qgmgr/pgmgr/hgmgr (build_limit()'s build_consumer() calls
|
|
* get_group(mgr, "all") for every consumer type our test configs leave
|
|
* NULL -- same minimal setup ../../lib/testsuite/test_limit.c's own
|
|
* test_build_limit_ok1() uses for build_limit() directly), plus a real
|
|
* limit plugin (limit_init()) so plugin.data is a real
|
|
* limit_plugin_data_t with privdata->limits configured with
|
|
* .datakiller = kill_limit -- the exact kvmgr shape bug-1685 crashed on.
|
|
* ---------------------------------------------------------------------- */
|
|
static void fixture_init(xce_scheduler_t* sched, xce_sched_plugin_t* plugin)
|
|
{
|
|
int rc;
|
|
|
|
memset(sched, 0, sizeof(*sched));
|
|
groupmgr_init(&(sched->core.ugmgr), 4, 4);
|
|
groupmgr_init(&(sched->core.qgmgr), 4, 4);
|
|
groupmgr_init(&(sched->core.pgmgr), 4, 4);
|
|
groupmgr_init(&(sched->core.hgmgr), 4, 4);
|
|
TEST_ASSERT_NOT_NULL(sched->core.ugmgr);
|
|
TEST_ASSERT_NOT_NULL(sched->core.qgmgr);
|
|
TEST_ASSERT_NOT_NULL(sched->core.pgmgr);
|
|
TEST_ASSERT_NOT_NULL(sched->core.hgmgr);
|
|
|
|
memset(plugin, 0, sizeof(*plugin));
|
|
plugin->sched = sched;
|
|
|
|
rc = limit_init(plugin);
|
|
TEST_ASSERT_EQUAL_INT(0, rc);
|
|
TEST_ASSERT_NOT_NULL(plugin->data);
|
|
}
|
|
|
|
/* A minimal, heap-allocated xce_limit_config_t: no per-consumer groups
|
|
* (per_user/per_queue/per_project/per_host all 0, users/queues/projects/
|
|
* hosts all NULL), so build_limit()'s build_consumer() falls back to
|
|
* get_group(mgr, "all") for every consumer -- the same "all"-group path
|
|
* ../../lib/testsuite/test_limit.c's own fixtures rely on. `name` is
|
|
* expected to be a string literal (build_limit() strdup()s it into the
|
|
* built xce_limit_t, so no ownership is transferred here). */
|
|
static xce_limit_config_t* make_limit_config(const char* name)
|
|
{
|
|
xce_limit_config_t* cfg = (xce_limit_config_t*)calloc(1, sizeof(*cfg));
|
|
xce_limit_val_t* val = (xce_limit_val_t*)calloc(1, sizeof(*val));
|
|
|
|
TEST_ASSERT_NOT_NULL(cfg);
|
|
TEST_ASSERT_NOT_NULL(val);
|
|
|
|
val->resname = "mem";
|
|
val->percent = 0;
|
|
val->val = 10.0;
|
|
|
|
cfg->name = (char*)name;
|
|
cfg->val = val;
|
|
|
|
return cfg;
|
|
}
|
|
|
|
static void free_limit_config(xce_limit_config_t* cfg)
|
|
{
|
|
FREEUP(cfg->val);
|
|
FREEUP(cfg);
|
|
}
|
|
|
|
/*
|
|
* THE regression: a config update with TWO xce_limit_config_t entries
|
|
* sharing the SAME name ("dup_limit") -- the exact duplicate-name
|
|
* collision bug-1685's live crash captured (rc=-17/-EEXIST at the crash
|
|
* frame, on/near the first limit of a 7-limit config). Pre-fix, this
|
|
* aborts the process (SIGABRT, "double free or corruption"); post-fix,
|
|
* limit_update_config() must return a negative rc cleanly, with no crash.
|
|
* Under ASAN, a double free is caught unconditionally regardless of
|
|
* whether this test also makes an explicit assertion about it -- the
|
|
* absence of an abort IS the proof (same rationale the task description
|
|
* for this test uses: "that's literally what caught the original bug").
|
|
*/
|
|
static void test_duplicate_limit_name_no_double_free(void)
|
|
{
|
|
xce_scheduler_t sched;
|
|
xce_sched_plugin_t plugin;
|
|
xce_limit_config_t* limits[2];
|
|
xce_conf4sched_t config;
|
|
int rc;
|
|
|
|
fixture_init(&sched, &plugin);
|
|
|
|
limits[0] = make_limit_config("dup_limit");
|
|
limits[1] = make_limit_config("dup_limit");
|
|
|
|
memset(&config, 0, sizeof(config));
|
|
config.nlimits = 2;
|
|
config.limits = limits;
|
|
|
|
rc = limit_update_config(&plugin, &config);
|
|
|
|
TEST_ASSERT_LESS_THAN_INT(0, rc);
|
|
TEST_ASSERT_EQUAL_INT(-EEXIST, rc);
|
|
|
|
free_limit_config(limits[0]);
|
|
free_limit_config(limits[1]);
|
|
}
|
|
|
|
/*
|
|
* Control case: a config update with DISTINCT limit names succeeds for
|
|
* both entries, proving the fix didn't just suppress freeing on failure
|
|
* in general -- it only stopped the redundant free on THIS specific
|
|
* (kvtab-key_add-failure) rollback path. Both built xce_limit_t values
|
|
* are now owned by privdata->limits (torn down, if at all, only by the
|
|
* kvmgr's own datakiller at process exit -- suite convention:
|
|
* ASAN_OPTIONS=detect_leaks=0, fixtures are never explicitly torn down,
|
|
* see meson.build).
|
|
*/
|
|
static void test_distinct_limit_names_both_succeed(void)
|
|
{
|
|
xce_scheduler_t sched;
|
|
xce_sched_plugin_t plugin;
|
|
xce_limit_config_t* limits[2];
|
|
xce_conf4sched_t config;
|
|
int rc;
|
|
|
|
fixture_init(&sched, &plugin);
|
|
|
|
limits[0] = make_limit_config("limit_a");
|
|
limits[1] = make_limit_config("limit_b");
|
|
|
|
memset(&config, 0, sizeof(config));
|
|
config.nlimits = 2;
|
|
config.limits = limits;
|
|
|
|
rc = limit_update_config(&plugin, &config);
|
|
|
|
TEST_ASSERT_EQUAL_INT(0, rc);
|
|
|
|
free_limit_config(limits[0]);
|
|
free_limit_config(limits[1]);
|
|
}
|
|
|
|
/* ---------------------------------------------------------------------- */
|
|
/* scheduler-sim-diagnostics spec, Requirement Set 1 (Limit Plugin
|
|
* Accessor), task 1: limit_get_configured_limits()/limit_get_usage_
|
|
* instances() (plugin.h, this file). Pure read accessors crossing the
|
|
* plugin-private-state boundary. These tests prove (a) a registered
|
|
* plugin's real limits/instances kvmgr handles come back non-NULL and
|
|
* genuinely iterable via iter_make()/iter_next() -- including yielding
|
|
* back the exact same limit that was configured, (b) a registered-but-
|
|
* empty plugin (limit_init() ran, but no limit/instance was ever added)
|
|
* still returns a non-NULL, empty-iterating handle rather than NULL, and
|
|
* (c) an unregistered plugin returns NULL from both accessors.
|
|
*
|
|
* fixture_init() (above) builds a real limit plugin via limit_init()
|
|
* directly (plugin_register() is stubbed to a no-op in this TU), so
|
|
* plugin->plugin_id/core->plugins are never populated by it -- these
|
|
* accessors' key_find(core->plugins, "xce_sched_limit", &plugin) lookup
|
|
* needs a real core->plugins kvmgr with the plugin actually key_add()'d
|
|
* into it under that exact name. The real plugin_register() (plugin.c)
|
|
* is not linked into this TU (scheduler_test_deps carries no scheduler-
|
|
* library dependency -- see this file's own header comment), so
|
|
* xce_sched_core_init() (core.c) can't be called either. The two helpers
|
|
* below replicate just the one piece of it these tests need: the same
|
|
* core->plugins kvmgr core.c's xce_sched_core_init() creates (same key
|
|
* type/sizing as its plugins_params), minus its .datakiller
|
|
* (plugin_unregister -- also unlinked here; NULL is safe since this
|
|
* suite never tears fixtures down, matching every other fixture's
|
|
* "ASAN_OPTIONS=detect_leaks=0" convention, meson.build).
|
|
*/
|
|
|
|
/* Registered case: core->plugins is a real kvmgr with the plugin key_add()'d
|
|
* into it under its real registration name, exactly as plugin_register()
|
|
* (plugin.c) itself would do -- so the accessors' key_find() success path
|
|
* is genuinely exercised, not just limit_init()'s privdata setup. */
|
|
static void fixture_register_plugin_in_core(xce_sched_core_t* core,
|
|
xce_sched_plugin_t* plugin)
|
|
{
|
|
kvmgr_params_t plugins_params = { .key_type = KVMGR_KEY_STRING,
|
|
.nslots = 16,
|
|
.delta = 8,
|
|
.datakiller = NULL };
|
|
int rc;
|
|
|
|
core->plugins = makeInstance(&xkvmgr_type, &plugins_params);
|
|
TEST_ASSERT_NOT_NULL(core->plugins);
|
|
|
|
plugin->name = "xce_sched_limit";
|
|
|
|
rc = key_add(core->plugins, (void*)plugin->name, (void*)plugin);
|
|
TEST_ASSERT_TRUE(rc >= 0);
|
|
}
|
|
|
|
/* Not-registered case: core->plugins is still a real, initialized
|
|
* (non-NULL) kvmgr -- matching the accessors' documented precondition
|
|
* that core is xce_sched_core_init()-initialized -- but the
|
|
* "xce_sched_limit" key is never added to it. This is the realistic
|
|
* not-registered scenario: core->plugins itself always exists once a
|
|
* core is initialized; only the specific plugin entry is ever missing. */
|
|
static void fixture_init_empty_plugin_registry(xce_sched_core_t* core)
|
|
{
|
|
kvmgr_params_t plugins_params = { .key_type = KVMGR_KEY_STRING,
|
|
.nslots = 16,
|
|
.delta = 8,
|
|
.datakiller = NULL };
|
|
|
|
core->plugins = makeInstance(&xkvmgr_type, &plugins_params);
|
|
TEST_ASSERT_NOT_NULL(core->plugins);
|
|
}
|
|
|
|
/* (c) plugin not registered -> NULL from both accessors. */
|
|
static void test_accessors_null_when_plugin_not_registered(void)
|
|
{
|
|
xce_scheduler_t sched;
|
|
xce_sched_plugin_t plugin;
|
|
var limits_handle;
|
|
var instances_handle;
|
|
|
|
fixture_init(&sched, &plugin);
|
|
fixture_init_empty_plugin_registry(&(sched.core));
|
|
|
|
limits_handle = limit_get_configured_limits(&(sched.core));
|
|
instances_handle = limit_get_usage_instances(&(sched.core));
|
|
|
|
TEST_ASSERT_NULL(limits_handle);
|
|
TEST_ASSERT_NULL(instances_handle);
|
|
}
|
|
|
|
/* (a) The observable case: registered plugin with a known configured
|
|
* limit -- limit_get_configured_limits()'s returned handle, iterated via
|
|
* iter_make()/iter_next(), yields that same limit back. Also confirms
|
|
* limit_get_usage_instances() reads a genuinely different field (not an
|
|
* alias of the limits handle) and, since no job has matched against this
|
|
* limit yet, is itself non-NULL but empty (Requirement 2.2's "no active
|
|
* usage instance yet"). */
|
|
static void test_get_configured_limits_handle_iterates_registered_limit(void)
|
|
{
|
|
xce_scheduler_t sched;
|
|
xce_sched_plugin_t plugin;
|
|
xce_limit_config_t* limits[1];
|
|
xce_conf4sched_t config;
|
|
var limits_handle;
|
|
var instances_handle;
|
|
iterator_t it;
|
|
xce_limit_t* found = NULL;
|
|
int rc;
|
|
|
|
fixture_init(&sched, &plugin);
|
|
fixture_register_plugin_in_core(&(sched.core), &plugin);
|
|
|
|
limits[0] = make_limit_config("observable_limit");
|
|
|
|
memset(&config, 0, sizeof(config));
|
|
config.nlimits = 1;
|
|
config.limits = limits;
|
|
|
|
rc = limit_update_config(&plugin, &config);
|
|
TEST_ASSERT_EQUAL_INT(0, rc);
|
|
|
|
limits_handle = limit_get_configured_limits(&(sched.core));
|
|
TEST_ASSERT_NOT_NULL(limits_handle);
|
|
|
|
rc = iter_make(&it, limits_handle);
|
|
TEST_ASSERT_TRUE(rc >= 0);
|
|
|
|
rc = iter_next(&it, (void**)&found);
|
|
TEST_ASSERT_TRUE(rc >= 0);
|
|
TEST_ASSERT_NOT_EQUAL(RTTI_ITER_NO_DATA, rc);
|
|
TEST_ASSERT_NOT_NULL(found);
|
|
TEST_ASSERT_EQUAL_STRING("observable_limit", found->name);
|
|
|
|
rc = iter_next(&it, (void**)&found);
|
|
TEST_ASSERT_EQUAL_INT(RTTI_ITER_NO_DATA, rc);
|
|
|
|
iter_destroy(&it);
|
|
|
|
instances_handle = limit_get_usage_instances(&(sched.core));
|
|
TEST_ASSERT_NOT_NULL(instances_handle);
|
|
TEST_ASSERT_TRUE(instances_handle != limits_handle);
|
|
|
|
{
|
|
void* item = NULL;
|
|
|
|
rc = iter_make(&it, instances_handle);
|
|
TEST_ASSERT_TRUE(rc >= 0);
|
|
|
|
rc = iter_next(&it, &item);
|
|
TEST_ASSERT_EQUAL_INT(RTTI_ITER_NO_DATA, rc);
|
|
|
|
iter_destroy(&it);
|
|
}
|
|
|
|
free_limit_config(limits[0]);
|
|
}
|
|
|
|
/* (b) plugin registered with zero limits: privdata->limits is a real,
|
|
* freshly created (limit_init()) kvmgr with nothing added to it yet --
|
|
* the accessor must still return a non-NULL handle (distinguishing
|
|
* "plugin present, zero limits" from "plugin absent" per Requirement
|
|
* 1.7), and iterating it must yield no elements. */
|
|
static void test_get_configured_limits_registered_with_zero_limits(void)
|
|
{
|
|
xce_scheduler_t sched;
|
|
xce_sched_plugin_t plugin;
|
|
var limits_handle;
|
|
iterator_t it;
|
|
void* item = NULL;
|
|
int rc;
|
|
|
|
fixture_init(&sched, &plugin);
|
|
fixture_register_plugin_in_core(&(sched.core), &plugin);
|
|
|
|
limits_handle = limit_get_configured_limits(&(sched.core));
|
|
TEST_ASSERT_NOT_NULL(limits_handle);
|
|
|
|
rc = iter_make(&it, limits_handle);
|
|
TEST_ASSERT_TRUE(rc >= 0);
|
|
|
|
rc = iter_next(&it, &item);
|
|
TEST_ASSERT_EQUAL_INT(RTTI_ITER_NO_DATA, rc);
|
|
|
|
iter_destroy(&it);
|
|
}
|
|
|
|
int main(void)
|
|
{
|
|
UNITY_BEGIN();
|
|
RUN_TEST(test_duplicate_limit_name_no_double_free);
|
|
RUN_TEST(test_distinct_limit_names_both_succeed);
|
|
RUN_TEST(test_accessors_null_when_plugin_not_registered);
|
|
RUN_TEST(test_get_configured_limits_handle_iterates_registered_limit);
|
|
RUN_TEST(test_get_configured_limits_registered_with_zero_limits);
|
|
return UNITY_END();
|
|
}
|