diff --git a/CLAUDE.md b/CLAUDE.md index dcbeb1af..895e6fec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -727,8 +727,8 @@ role, not platform*, for the rule and the two-implementations-in-one-build cavea exhaustion site if a handler is installed. - **Slot-walk synchronisation.** Every pool's Create / Destroy wraps its slot probe in the `SolidSyslog_LockConfig` / `SolidSyslog_UnlockConfig` injection pair. Single-task targets get the - no-op default; multi-task targets wire `taskENTER_CRITICAL` (FreeRTOS), `pthread_mutex_lock` - (POSIX), `EnterCriticalSection` (Windows), etc. + no-op default. Cleanup runs inside the lock, so a multi-task target must install one that + tolerates blocking and is not a SolidSyslog Mutex; `SolidSyslogConfigLock.h` states the contract. - **Shared helper.** `Core/Source/SolidSyslogPoolAllocator.{h,c}` (TU-internal) owns the three-operation contract (`AcquireFirstFree`, `FreeIfInUse`, `IndexIsValid`) every pool class reuses. No class re-implements the slot walk. diff --git a/Core/Interface/SolidSyslogConfigLock.h b/Core/Interface/SolidSyslogConfigLock.h index fa6f2eb0..b4cb8477 100644 --- a/Core/Interface/SolidSyslogConfigLock.h +++ b/Core/Interface/SolidSyslogConfigLock.h @@ -1,6 +1,6 @@ /** @file - * The config-time critical-section injection pair guarding every pool - * Create/Destroy slot-walk; the no-op default suits single-task setup. */ + * The config-time lock injection pair guarding every pool Create/Destroy + * slot-walk; the no-op default suits single-task setup. */ #ifndef SOLIDSYSLOGCONFIGLOCK_H #define SOLIDSYSLOGCONFIGLOCK_H @@ -8,22 +8,32 @@ SOLIDSYSLOG_EXTERN_C_BEGIN - /** Critical-section enter/leave callback wrapping every pool Create/Destroy - * slot-walk. Single-task targets need none (the default is a no-op); - * multi-task targets wire taskENTER_CRITICAL (FreeRTOS), - * pthread_mutex_lock on a static mutex (POSIX), EnterCriticalSection - * (Windows), or a spinlock pair. Because this guards the pool walks, it is - * the one synchronisation primitive the Mutex and AtomicCounter pools can - * use for their own walks without a chicken-and-egg dependency on - * themselves. */ - typedef void (*SolidSyslogConfigLockFunction)(void); + /** Lock/unlock callback pair wrapping every pool Create/Destroy slot-walk, + * each passed the context installed with it. The default is a no-op, which + * suits single-task setup. + * + * Where Create and Destroy can run concurrently, install a mutex supplied + * by the host system. Destroy releases the instance's resources inside + * this lock and may block, so the primitive must permit blocking while + * held; one that disables interrupts or spins will deadlock. + * + * A SolidSyslog Mutex is not a valid choice: its Create walks a pool under + * this lock. Requiring a host primitive is also what allows the Mutex and + * AtomicCounter pools to use this pair for their own walks. */ + typedef void (*SolidSyslogConfigLockFunction)(void* context); /** Install the config-lock pair, applied setup-time before any Create. - * Single global slot, not synchronised against concurrent installs. Both - * handlers are set together; NULL on either side restores that side's - * no-op default. */ - /* NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- deliberate pair API: lock and unlock are installed together and conceptually inseparable; matches SolidSyslog_SetErrorHandler's pair shape */ - void SolidSyslog_SetConfigLock(SolidSyslogConfigLockFunction lockFn, SolidSyslogConfigLockFunction unlockFn); + * Single global slot, not synchronised against concurrent installs. All + * three are set together; @p context is passed to both callbacks + * unchanged, and NULL on either function restores that side's no-op + * default. */ + /* NOLINTBEGIN(bugprone-easily-swappable-parameters) -- deliberate pair API: lock and unlock are installed together and conceptually inseparable; matches SolidSyslog_SetErrorHandler's pair shape */ + void SolidSyslog_SetConfigLock( + SolidSyslogConfigLockFunction lockFn, + SolidSyslogConfigLockFunction unlockFn, + void* context + ); + /* NOLINTEND(bugprone-easily-swappable-parameters) */ void SolidSyslog_LockConfig(void); void SolidSyslog_UnlockConfig(void); diff --git a/Core/Source/SolidSyslogConfigLock.c b/Core/Source/SolidSyslogConfigLock.c index 0698bf5f..d684a15e 100644 --- a/Core/Source/SolidSyslogConfigLock.c +++ b/Core/Source/SolidSyslogConfigLock.c @@ -2,16 +2,24 @@ #include -static void ConfigLock_NoOp(void) +static void ConfigLock_NoOp(void* context) { + (void) context; } static SolidSyslogConfigLockFunction ConfigLock_Lock = ConfigLock_NoOp; static SolidSyslogConfigLockFunction ConfigLock_Unlock = ConfigLock_NoOp; +static void* ConfigLock_Context = NULL; -// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- deliberate pair API: lock and unlock are installed together and conceptually inseparable; matches SolidSyslog_SetErrorHandler's pair shape -void SolidSyslog_SetConfigLock(SolidSyslogConfigLockFunction lockFn, SolidSyslogConfigLockFunction unlockFn) +// NOLINTBEGIN(bugprone-easily-swappable-parameters) -- deliberate pair API: lock and unlock are installed together and conceptually inseparable; matches SolidSyslog_SetErrorHandler's pair shape +void SolidSyslog_SetConfigLock( + SolidSyslogConfigLockFunction lockFn, + SolidSyslogConfigLockFunction unlockFn, + void* context +) +// NOLINTEND(bugprone-easily-swappable-parameters) { + ConfigLock_Context = context; if (lockFn == NULL) { ConfigLock_Lock = ConfigLock_NoOp; @@ -32,10 +40,10 @@ void SolidSyslog_SetConfigLock(SolidSyslogConfigLockFunction lockFn, SolidSyslog void SolidSyslog_LockConfig(void) { - ConfigLock_Lock(); + ConfigLock_Lock(ConfigLock_Context); } void SolidSyslog_UnlockConfig(void) { - ConfigLock_Unlock(); + ConfigLock_Unlock(ConfigLock_Context); } diff --git a/Tests/SolidSyslogConfigLockTest.cpp b/Tests/SolidSyslogConfigLockTest.cpp index 17678c26..b6cf4e58 100644 --- a/Tests/SolidSyslogConfigLockTest.cpp +++ b/Tests/SolidSyslogConfigLockTest.cpp @@ -8,14 +8,28 @@ using namespace CososoTesting; static int testLockCallCount; static int testUnlockCallCount; +static void* testLockContext; +static void* testUnlockContext; -static void TestLock() +static void TestLockRecordingContext(void* context) { + testLockContext = context; +} + +static void TestUnlockRecordingContext(void* context) +{ + testUnlockContext = context; +} + +static void TestLock(void* context) +{ + (void) context; testLockCallCount++; } -static void TestUnlock() +static void TestUnlock(void* context) { + (void) context; testUnlockCallCount++; } @@ -64,11 +78,31 @@ TEST(SolidSyslogConfigLock, InstalledUnlockFunctionIsCalledByUnlockConfig) CALLED_FAKE(ConfigLockFake_Unlock, ONCE); } +TEST(SolidSyslogConfigLock, LockFunctionReceivesInstalledContext) +{ + int context = 0; + + SolidSyslog_SetConfigLock(TestLockRecordingContext, nullptr, &context); + SolidSyslog_LockConfig(); + + POINTERS_EQUAL(&context, testLockContext); +} + +TEST(SolidSyslogConfigLock, UnlockFunctionReceivesInstalledContext) +{ + int context = 0; + + SolidSyslog_SetConfigLock(nullptr, TestUnlockRecordingContext, &context); + SolidSyslog_UnlockConfig(); + + POINTERS_EQUAL(&context, testUnlockContext); +} + TEST(SolidSyslogConfigLock, SetConfigLockWithNullLockRestoresDefault) { - SolidSyslog_SetConfigLock(TestLock, TestUnlock); + SolidSyslog_SetConfigLock(TestLock, TestUnlock, nullptr); - SolidSyslog_SetConfigLock(nullptr, TestUnlock); + SolidSyslog_SetConfigLock(nullptr, TestUnlock, nullptr); SolidSyslog_LockConfig(); CALLED_FUNCTION(testLock, NEVER); @@ -76,9 +110,9 @@ TEST(SolidSyslogConfigLock, SetConfigLockWithNullLockRestoresDefault) TEST(SolidSyslogConfigLock, SetConfigLockWithNullUnlockRestoresDefault) { - SolidSyslog_SetConfigLock(TestLock, TestUnlock); + SolidSyslog_SetConfigLock(TestLock, TestUnlock, nullptr); - SolidSyslog_SetConfigLock(TestLock, nullptr); + SolidSyslog_SetConfigLock(TestLock, nullptr, nullptr); SolidSyslog_UnlockConfig(); CALLED_FUNCTION(testUnlock, NEVER); diff --git a/Tests/Support/ConfigLockFake.c b/Tests/Support/ConfigLockFake.c index 7f8d6886..69789794 100644 --- a/Tests/Support/ConfigLockFake.c +++ b/Tests/Support/ConfigLockFake.c @@ -7,13 +7,15 @@ static int lockCallCount; static int unlockCallCount; -static void Lock(void) +static void Lock(void* context) { + (void) context; lockCallCount++; } -static void Unlock(void) +static void Unlock(void* context) { + (void) context; unlockCallCount++; } @@ -21,12 +23,12 @@ void ConfigLockFake_Install(void) { lockCallCount = 0; unlockCallCount = 0; - SolidSyslog_SetConfigLock(Lock, Unlock); + SolidSyslog_SetConfigLock(Lock, Unlock, NULL); } void ConfigLockFake_Uninstall(void) { - SolidSyslog_SetConfigLock(NULL, NULL); + SolidSyslog_SetConfigLock(NULL, NULL, NULL); } int ConfigLockFake_LockCallCount(void) diff --git a/docs/porting.md b/docs/porting.md index 66863391..d0812f74 100644 --- a/docs/porting.md +++ b/docs/porting.md @@ -133,26 +133,20 @@ The pool allocator wraps each slot claim and release in the `AcquireFirstFree` locks per-slot around the claim, `FreeIfInUse` locks around the release, so an adapter's `_Create` / `_Destroy` inherit the synchronisation for free and never lock themselves (which is why the example above has no lock call). -Single-task setup gets the no-op default and pays nothing. On a multi-task or multi-core target -where setup races, install the pair once with `SolidSyslog_SetConfigLock(...)`: -`taskENTER_CRITICAL` / `taskEXIT_CRITICAL` (FreeRTOS), a static `pthread_mutex_t` -(POSIX), `EnterCriticalSection` / `LeaveCriticalSection` (Windows), or a spinlock. -This is the only synchronisation primitive the pools use for their own walks. +Single-task setup gets the no-op default and pays nothing. Where setup races, the +integrator installs the pair once with `SolidSyslog_SetConfigLock(...)`; +[`SolidSyslogConfigLock.h`](api/SolidSyslogConfigLock_8h.md) states what that lock +must be. This is the only synchronisation primitive the pools use for their own +walks. + +Your `Destroy` runs inside it, so cleanup may block and the installed lock must +allow it. ## Invariants every adapter must honour - Idempotent `Close` / `Destroy`. No leak on a partial `Open` failure, no double-free if `Close` and `Destroy` are both called. Release each resource exactly once and null the handle. -- Cleanup runs under the config lock. `FreeIfInUse` holds - `SolidSyslog_LockConfig()` across your cleanup callback, so whatever `Destroy` - does to release a resource happens inside whichever primitive the integrator - installed — a FreeRTOS critical section, at the recommendation above. Blocking - there is not safe: a mutex may not be taken inside `taskENTER_CRITICAL`, and - work that needs another task to run cannot complete while interrupts are off. - The shipped lwIP TCP stream does block this way, tracked as - [#754](https://github.com/cososo-ltd/solid-syslog/issues/754); until that is - resolved, keep teardown in your own adapter non-blocking. - Never free injected handles. An adapter frees only what it created. Handles the integrator passed in (a certificate, an RNG, a caller's socket) are borrowed; the owner frees them. The same applies to an upstream library's