From f155b1ef9771985fef7dc4905249b917f0b57450 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 7 May 2026 13:55:42 +0000 Subject: [PATCH] feat: xce_disable_hook() + _Atomic hook_fn_t + NULL-slot reuse (P1-2a) - hook_item_t.fn changed to _Atomic hook_fn_t (C11 stdatomic) - xce_disable_hook(): atomically stores NULL into the slot fn field; returns 0 on success, -ENOENT if fn not found, -EINVAL on bad args - xce_add_hook(): scans for NULL (disabled) slots before allocating; uses atomic_store_explicit(release) when writing fn pointer - xce_run_hooks(): atomic_load_explicit(acquire) + NULL guard so disabled slots are silently skipped without disrupting the loop - Add src/lib/testsuite/test_hooks.c with 3 TDD tests: test_disable_hook_stops_invocation test_disable_hook_returns_enoent_if_not_found test_add_hook_reuses_null_slot - Register test_hooks in meson.build and test.c runner Required by ha_demote_to_standby() to atomically disable persistence hooks when the controller steps down from primary, preventing unbounded hook list growth across promote/demote cycles (resolution A6-015, A7-004, A7-016). --- src/include/hook.h | 6 +- src/lib/hook.c | 52 ++++++++++++++- src/lib/meson.build | 1 + src/lib/testsuite/test.c | 2 + src/lib/testsuite/test_hooks.c | 112 +++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 src/lib/testsuite/test_hooks.c diff --git a/src/include/hook.h b/src/include/hook.h index bf6c7cc0..8d97af9c 100644 --- a/src/include/hook.h +++ b/src/include/hook.h @@ -3,6 +3,7 @@ #define __HOOK_H__ 1 #include +#include #include #include "xlist.h" @@ -12,8 +13,8 @@ typedef int (*hook_fn_t)(int, int, int, void*, va_list); typedef struct { - hook_fn_t fn; - void* data; + _Atomic hook_fn_t fn; + void* data; } hook_item_t; typedef struct { @@ -23,6 +24,7 @@ typedef struct { extern int xce_hook_init(hooksys_t*, int); extern int xce_add_hook(hooksys_t*, int, hook_fn_t, void*); +extern int xce_disable_hook(hooksys_t*, int, hook_fn_t); extern int xce_run_hooks(hooksys_t*, int, int, int, ...); extern void xce_hook_fini(hooksys_t*); diff --git a/src/lib/hook.c b/src/lib/hook.c index 8402afff..00d23c2a 100644 --- a/src/lib/hook.c +++ b/src/lib/hook.c @@ -1,4 +1,5 @@ #include +#include #include #include @@ -41,19 +42,60 @@ int xce_add_hook(hooksys_t* s, int typ, hook_fn_t fn, void* data) return -EINVAL; } + /* Scan for a disabled (NULL-fn) slot of the same type to reuse */ + { + xlist_t* l = &s->hooks[typ]; + int i; + for (i = 0; i < l->count; i++) { + hook_item_t* slot = (hook_item_t*)l->data[i]; + if (!slot) continue; + hook_fn_t cur = + atomic_load_explicit(&slot->fn, memory_order_acquire); + if (cur == NULL) { + slot->data = data; + atomic_store_explicit(&slot->fn, fn, memory_order_release); + return 0; + } + } + } + + /* No reusable slot — allocate new */ it = (hook_item_t*)calloc(1, sizeof(*it)); if (it == NULL) { return -ENOMEM; } - it->fn = fn; it->data = data; + atomic_store_explicit(&it->fn, fn, memory_order_release); xlist_store(&(s->hooks[typ]), it); return 0; } +int xce_disable_hook(hooksys_t* s, int typ, hook_fn_t fn) +{ + xlist_t* l; + int i; + + if (!s || !fn || typ < 0 || typ >= s->maxtypes) { + return -EINVAL; + } + + l = &s->hooks[typ]; + for (i = 0; i < l->count; i++) { + hook_item_t* it = (hook_item_t*)l->data[i]; + if (!it) continue; + hook_fn_t cur = atomic_load_explicit(&it->fn, memory_order_acquire); + if (cur == fn) { + atomic_store_explicit(&it->fn, (hook_fn_t)NULL, + memory_order_release); + return 0; + } + } + return -ENOENT; +} + /* HOOK_BEFORE_EVENT returns * negative value will cause * fail @@ -76,14 +118,20 @@ int xce_run_hooks(hooksys_t* s, int type, int situation, int detail, ...) for (i = 0; i < l->count; i++) { hook_item_t* it = l->data[i]; + hook_fn_t local_fn; va_list ap; if (it == NULL) { continue; } + local_fn = atomic_load_explicit(&it->fn, memory_order_acquire); + if (!local_fn) { + continue; /* disabled slot */ + } + va_start(ap, detail); - rc = (*(it->fn))(type, situation, detail, it->data, ap); + rc = local_fn(type, situation, detail, it->data, ap); va_end(ap); if ((situation == HOOK_BEFORE_EVENT) && (rc < 0)) { diff --git a/src/lib/meson.build b/src/lib/meson.build index 486853da..c63b6d63 100644 --- a/src/lib/meson.build +++ b/src/lib/meson.build @@ -165,6 +165,7 @@ libxce_test_src = ['testsuite/test.c', 'testsuite/test_request.c', 'testsuite/test_zarray_cow.c', 'testsuite/test_kvmgr_capture.c', + 'testsuite/test_hooks.c', ] libxce_test_deps = [libxce_deps, librtti_dep, unity_dep, libxce_dep] diff --git a/src/lib/testsuite/test.c b/src/lib/testsuite/test.c index 4ef9aff1..2cb1fe96 100644 --- a/src/lib/testsuite/test.c +++ b/src/lib/testsuite/test.c @@ -31,6 +31,7 @@ extern void test_wobj(void); extern void test_request(void); extern void test_zarray_cow(void); extern void test_kvmgr_capture(void); +extern void test_hooks(void); void setUp(void) { return; } @@ -131,6 +132,7 @@ int main(int argc, char* argv[]) TEST(test_request); TEST(test_zarray_cow); TEST(test_kvmgr_capture); + TEST(test_hooks); return UNITY_END(); } diff --git a/src/lib/testsuite/test_hooks.c b/src/lib/testsuite/test_hooks.c new file mode 100644 index 00000000..14fafe42 --- /dev/null +++ b/src/lib/testsuite/test_hooks.c @@ -0,0 +1,112 @@ +#include +#include +#include + +#include "hook.h" +#include + +/* Shared call counter for hook tests */ +static int s_call_count = 0; + +static int counting_fn(int type, int sit, int det, void* data, va_list ap) +{ + (void)type; + (void)sit; + (void)det; + (void)ap; + (*(int*)data)++; + return 0; +} + +/* --- test cases --- */ + +static void test_disable_hook_stops_invocation(void) +{ + hooksys_t hs; + int rc; + + s_call_count = 0; + + rc = xce_hook_init(&hs, 2 /* maxtypes */); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = xce_add_hook(&hs, HOOK_AFTER_EVENT, counting_fn, &s_call_count); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* Before disable: hook fires once */ + xce_run_hooks(&hs, HOOK_AFTER_EVENT, 0, 0); + TEST_ASSERT_EQUAL_INT(1, s_call_count); + + /* Disable the hook */ + rc = xce_disable_hook(&hs, HOOK_AFTER_EVENT, counting_fn); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* After disable: hook must not fire */ + xce_run_hooks(&hs, HOOK_AFTER_EVENT, 0, 0); + TEST_ASSERT_EQUAL_INT(1, s_call_count); /* unchanged */ + + xce_hook_fini(&hs); + printf("test_disable_hook_stops_invocation: PASS\n"); +} + +static void test_disable_hook_returns_enoent_if_not_found(void) +{ + hooksys_t hs; + int rc; + + rc = xce_hook_init(&hs, 2); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* Disabling a hook never added must return non-zero (ENOENT) */ + rc = xce_disable_hook(&hs, HOOK_AFTER_EVENT, counting_fn); + TEST_ASSERT_NOT_EQUAL(0, rc); + + xce_hook_fini(&hs); + printf("test_disable_hook_returns_enoent_if_not_found: PASS\n"); +} + +static void test_add_hook_reuses_null_slot(void) +{ + hooksys_t hs; + int rc; + int count_before_reuse; + + s_call_count = 0; + + rc = xce_hook_init(&hs, 2); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = xce_add_hook(&hs, HOOK_AFTER_EVENT, counting_fn, &s_call_count); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* Record list length after first add */ + count_before_reuse = hs.hooks[HOOK_AFTER_EVENT].count; + TEST_ASSERT_EQUAL_INT(1, count_before_reuse); + + /* Disable the slot — fn becomes NULL atomically */ + rc = xce_disable_hook(&hs, HOOK_AFTER_EVENT, counting_fn); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* Re-add the same fn — must reuse the NULL slot, not grow the list */ + rc = xce_add_hook(&hs, HOOK_AFTER_EVENT, counting_fn, &s_call_count); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(count_before_reuse, hs.hooks[HOOK_AFTER_EVENT].count); + + /* Fire: must get exactly 1 call (not 2 if slot was duplicated) */ + xce_run_hooks(&hs, HOOK_AFTER_EVENT, 0, 0); + TEST_ASSERT_EQUAL_INT(1, s_call_count); + + xce_hook_fini(&hs); + printf("test_add_hook_reuses_null_slot: PASS\n"); +} + +/* --- suite entry point --- */ + +void test_hooks(void) +{ + printf("\n>> hook system tests\n\n"); + + RUN_TEST(test_disable_hook_stops_invocation); + RUN_TEST(test_disable_hook_returns_enoent_if_not_found); + RUN_TEST(test_add_hook_reuses_null_slot); +}