diff --git a/Makefile b/Makefile index b80a8f6..c3526d0 100644 --- a/Makefile +++ b/Makefile @@ -191,8 +191,12 @@ tests/test_ptools_params: $(UNIT_DIR)/test_ptools_params.c $(LIBDIR)/ptools.c $( tests/test_pfs_lock_ordering: $(UNIT_DIR)/test_pfs_lock_ordering.c $(CC) $(TEST_CFLAGS) $(CFLAGS) -o $@ $< -lpthread -tests/test_ptask_free: $(UNIT_DIR)/test_ptask_free.c - $(CC) $(TEST_CFLAGS) $(CFLAGS) -o $@ $< -lpthread +tests/test_ptask_free: $(UNIT_DIR)/test_ptask_free.c $(LIBDIR)/ptask_free.c + $(CC) $(TEST_CFLAGS) $(CFLAGS) -o $@ $^ \ + -Wl,--wrap=pthread_mutex_lock \ + -Wl,--wrap=pthread_mutex_unlock \ + -Wl,--wrap=pmem_free \ + -lpthread tests/test_prun: $(UNIT_DIR)/test_prun.c $(LIBDIR)/prun.c $(LIBDIR)/pdbg.c $(LIBDIR)/pmem.c $(LIBDIR)/putil.c $(LIBDIR)/ppath.c tests/stubs/test_stubs.c $(CC) -D_POSIX_C_SOURCE=199309L $(CFLAGS) -o $@ $^ \ diff --git a/pclsync/ptask.c b/pclsync/ptask.c index 07a32e9..12918e6 100644 --- a/pclsync/ptask.c +++ b/pclsync/ptask.c @@ -48,20 +48,12 @@ #include "pstatus.h" #include "psys.h" #include "ptask.h" +#include "ptask_free_internal.h" #include "ptree.h" #include "pupload.h" #define get_len(t) (sizeof(t) - offsetof(t, request)) -#define PSYNC_TASK_STATUS_RUNNING 0 -#define PSYNC_TASK_STATUS_READY 1 -#define PSYNC_TASK_STATUS_DONE 2 -#define PSYNC_TASK_STATUS_RETURNED 3 - -// #define PSYNC_WAIT_ANYBODY -1 // unused, but may be important later -#define PSYNC_WAIT_NOBODY -2 -#define PSYNC_WAIT_FREED -3 - #define TASK_TYPE_EXIT 0 #define TASK_TYPE_FILE_DWL 1 #define TASK_TYPE_FILE_DWL_NM 2 @@ -163,22 +155,6 @@ typedef struct { int fd; } download_context_t; -struct psync_task_t_ { - psync_task_callback_t callback; - void *param; - pthread_cond_t cond; - int id; - int status; -}; - -struct psync_task_manager_t_ { - pthread_mutex_t mutex; - int taskcnt; - int refcnt; - int waitfor; - struct psync_task_t_ tasks[]; -}; - static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; static int running = 0; static int sockd = INVALID_SOCKET; @@ -845,35 +821,6 @@ static int task_send_async(const void *task, size_t len) { return ret; } -static void psync_task_destroy(psync_task_manager_t tm) { - int i; - for (i = 0; i < tm->taskcnt; i++) - pthread_cond_destroy(&tm->tasks[i].cond); - pthread_mutex_destroy(&tm->mutex); - pmem_free(PMEM_SUBSYS_OTHER, tm); -} - -static void psync_task_dec_refcnt(psync_task_manager_t tm) { - int refcnt; - pthread_mutex_lock(&tm->mutex); - refcnt = --tm->refcnt; - pthread_mutex_unlock(&tm->mutex); - if (!refcnt) - psync_task_destroy(tm); -} - -static psync_task_manager_t psync_get_manager_of_task(struct psync_task_t_ *t) { - return (psync_task_manager_t)(((char *)(t - t->id)) - - offsetof(struct psync_task_manager_t_, tasks)); -} - -static void psync_task_entry(void *ptr) { - struct psync_task_t_ *t; - t = (struct psync_task_t_ *)ptr; - t->callback(ptr, t->param); - psync_task_dec_refcnt(psync_get_manager_of_task(t)); -} - void ptask_ldir_mk(psync_syncid_t syncid, psync_folderid_t folderid, psync_folderid_t localfolderid) { @@ -1181,28 +1128,6 @@ void *psync_task_papi_result(psync_task_manager_t tm, int id) { return ret; } -void psync_task_free(psync_task_manager_t tm) { - int refcnt, i; - pthread_mutex_lock(&tm->mutex); - if (tm->refcnt == 1) { - pthread_mutex_unlock(&tm->mutex); - psync_task_destroy(tm); - } - else { - tm->waitfor = PSYNC_WAIT_FREED; - for (i = 0; i < tm->taskcnt; i++) - if (tm->tasks[i].status == PSYNC_TASK_STATUS_READY) { - tm->tasks[i].status = PSYNC_TASK_STATUS_RETURNED; - pthread_cond_signal(&tm->tasks[i].cond); - } - refcnt = --tm->refcnt; - pthread_mutex_unlock(&tm->mutex); - if (!refcnt) { - psync_task_destroy(tm); - } - } -} - int psync_task_complete(void *h, void *data) { psync_task_manager_t tm; struct psync_task_t_ *t; diff --git a/pclsync/ptask_free.c b/pclsync/ptask_free.c new file mode 100644 index 0000000..463065c --- /dev/null +++ b/pclsync/ptask_free.c @@ -0,0 +1,72 @@ +/* + * ptask_free.c — psync_task_manager_t lifecycle: destroy and free. + * + * Extracted from ptask.c as a separately compilable unit so that + * tests/unit-tests/test_ptask_free.c can link against this file alone, + * without dragging in ptask.c's heavyweight dependencies (papi, psql, + * pdeflate, …). + * + * Fix: b92a389 — mutex is held during the refcnt check in all paths. + */ + +#include +#include + +#include "pmem.h" +#include "ptask_free_internal.h" + +/* ------------------------------------------------------------------ */ +/* Static helpers */ +/* ------------------------------------------------------------------ */ + +static void psync_task_destroy(psync_task_manager_t tm) { + int i; + for (i = 0; i < tm->taskcnt; i++) + pthread_cond_destroy(&tm->tasks[i].cond); + pthread_mutex_destroy(&tm->mutex); + pmem_free(PMEM_SUBSYS_OTHER, tm); +} + +static void psync_task_dec_refcnt(psync_task_manager_t tm) { + int refcnt; + pthread_mutex_lock(&tm->mutex); + refcnt = --tm->refcnt; + pthread_mutex_unlock(&tm->mutex); + if (!refcnt) + psync_task_destroy(tm); +} + +/* ------------------------------------------------------------------ */ +/* Thread entry-point (non-static: referenced from psync_task_run_tasks + * in ptask.c via the forward declaration in ptask_free_internal.h) */ +/* ------------------------------------------------------------------ */ + +void psync_task_entry(void *ptr) { + struct psync_task_t_ *t = (struct psync_task_t_ *)ptr; + t->callback(ptr, t->param); + psync_task_dec_refcnt(psync_get_manager_of_task(t)); +} + +/* ------------------------------------------------------------------ */ +/* Public API */ +/* ------------------------------------------------------------------ */ + +void psync_task_free(psync_task_manager_t tm) { + int refcnt, i; + pthread_mutex_lock(&tm->mutex); + if (tm->refcnt == 1) { + pthread_mutex_unlock(&tm->mutex); + psync_task_destroy(tm); + } else { + tm->waitfor = PSYNC_WAIT_FREED; + for (i = 0; i < tm->taskcnt; i++) + if (tm->tasks[i].status == PSYNC_TASK_STATUS_READY) { + tm->tasks[i].status = PSYNC_TASK_STATUS_RETURNED; + pthread_cond_signal(&tm->tasks[i].cond); + } + refcnt = --tm->refcnt; + pthread_mutex_unlock(&tm->mutex); + if (!refcnt) + psync_task_destroy(tm); + } +} diff --git a/pclsync/ptask_free_internal.h b/pclsync/ptask_free_internal.h new file mode 100644 index 0000000..80ab024 --- /dev/null +++ b/pclsync/ptask_free_internal.h @@ -0,0 +1,75 @@ +/* + * ptask_free_internal.h — internal layout of psync_task_manager_t. + * + * Included by: + * - pclsync/ptask_free.c (production lifecycle code) + * - pclsync/ptask.c (rest of the task subsystem) + * - tests/unit-tests/test_ptask_free.c (struct-level access in tests) + * + * NOT part of the public API. Do not include from general application code. + */ +#ifndef PTASK_FREE_INTERNAL_H +#define PTASK_FREE_INTERNAL_H + +#include +#include + +/* + * Provide minimal forward declarations when this header is included + * standalone (e.g. from the test). When ptask.h has already been + * included its include guard (_PSYNC_TASKS_H) suppresses the duplicates. + */ +#ifndef _PSYNC_TASKS_H +typedef void (*psync_task_callback_t)(void *, void *); +struct psync_task_manager_t_; +typedef struct psync_task_manager_t_ *psync_task_manager_t; +#endif + +/* Task status values */ +#define PSYNC_TASK_STATUS_RUNNING 0 +#define PSYNC_TASK_STATUS_READY 1 +#define PSYNC_TASK_STATUS_DONE 2 +#define PSYNC_TASK_STATUS_RETURNED 3 + +/* waitfor sentinel values */ +// #define PSYNC_WAIT_ANYBODY -1 /* unused, but may be useful later */ +#define PSYNC_WAIT_NOBODY -2 +#define PSYNC_WAIT_FREED -3 + +struct psync_task_t_ { + psync_task_callback_t callback; + void *param; + pthread_cond_t cond; + int id; + int status; +}; + +struct psync_task_manager_t_ { + pthread_mutex_t mutex; + int taskcnt; + int refcnt; + int waitfor; + struct psync_task_t_ tasks[]; +}; + +/* + * Helper: given a pointer to an individual task, return the owning manager. + * Declared static inline so both ptask.c and ptask_free.c can use it + * without any linkage conflict. + */ +static inline psync_task_manager_t +psync_get_manager_of_task(struct psync_task_t_ *t) { + return (psync_task_manager_t)(((char *)(t - t->id)) - + offsetof(struct psync_task_manager_t_, tasks)); +} + +/* Defined in ptask_free.c; declared in ptask.h for normal callers. */ +void psync_task_free(psync_task_manager_t tm); + +/* + * psync_task_entry is defined in ptask_free.c and used as a thread + * entry-point in psync_task_run_tasks (ptask.c). + */ +void psync_task_entry(void *ptr); + +#endif /* PTASK_FREE_INTERNAL_H */ diff --git a/tests/unit-tests/test_ptask_free.c b/tests/unit-tests/test_ptask_free.c index 1dcd921..95c867e 100644 --- a/tests/unit-tests/test_ptask_free.c +++ b/tests/unit-tests/test_ptask_free.c @@ -6,262 +6,224 @@ * 2. refcnt>1, last ref: lock acquired, refcnt decremented, destroy called * 3. refcnt>1, not last ref: refcnt decremented, destroy NOT called * 4. READY tasks get signaled (status→RETURNED) when freed with refcnt>1 + * 5. mutex is acquired before the refcnt check in all paths * - * The mutex is held during the refcnt check in all paths — the core fix. - * We verify this by intercepting pthread_mutex_lock/unlock with counters - * and confirming lock is held before destroy is invoked. + * Links against the real pclsync/ptask_free.c (production code). + * Uses --wrap linker flags to intercept pthread_mutex_lock/unlock and + * pmem_free so we can observe lock discipline and detect destroy calls + * without reimplementing any production logic inline. */ +#define _POSIX_C_SOURCE 200809L #include -#include #include #include #include +/* Internal struct layout — exposes psync_task_manager_t_ for make_tm() */ +#include "ptask_free_internal.h" +#include "pmem.h" + /* ------------------------------------------------------------------ */ -/* Intercept controls */ +/* __wrap / __real declarations */ /* ------------------------------------------------------------------ */ + +int __real_pthread_mutex_lock(pthread_mutex_t *m); +int __real_pthread_mutex_unlock(pthread_mutex_t *m); +void __real_pmem_free(pmem_subsystem_t subsystem, void *ptr); + +/* ------------------------------------------------------------------ */ +/* Intercept state */ +/* ------------------------------------------------------------------ */ + static int g_lock_calls = 0; static int g_unlock_calls = 0; -static int g_destroy_calls = 0; -static int g_free_calls = 0; -static int g_lock_held_at_destroy = 0; /* was lock held when destroy fired? */ +static int g_destroy_calls = 0; /* incremented by __wrap_pmem_free */ +static int g_lock_depth = 0; +static int g_lock_held_at_destroy = 0; /* 1 = mutex was unlocked when destroy fired */ /* ------------------------------------------------------------------ */ -/* Inline struct replica (mirrors ptask.c exactly) */ -/* ------------------------------------------------------------------ */ -#define PSYNC_TASK_STATUS_RUNNING 0 -#define PSYNC_TASK_STATUS_READY 1 -#define PSYNC_TASK_STATUS_DONE 2 -#define PSYNC_TASK_STATUS_RETURNED 3 - -#define PSYNC_WAIT_NOBODY -2 -#define PSYNC_WAIT_FREED -3 - -typedef void (*psync_task_callback_t)(void *, void *); - -struct psync_task_t_ { - psync_task_callback_t callback; - void *param; - pthread_cond_t cond; - int id; - int status; -}; - -struct psync_task_manager_t_ { - pthread_mutex_t mutex; - int taskcnt; - int refcnt; - int waitfor; - struct psync_task_t_ tasks[]; -}; - -typedef struct psync_task_manager_t_ *psync_task_manager_t; - -/* ------------------------------------------------------------------ */ -/* Mock implementations */ +/* Wrap implementations */ /* ------------------------------------------------------------------ */ -/* Track lock depth so we know if lock is held when destroy fires */ -static int g_lock_depth = 0; - -static int mock_mutex_lock(pthread_mutex_t *m) { - g_lock_calls++; - g_lock_depth++; - return pthread_mutex_lock(m); +int __wrap_pthread_mutex_lock(pthread_mutex_t *m) { + g_lock_calls++; + g_lock_depth++; + return __real_pthread_mutex_lock(m); } -static int mock_mutex_unlock(pthread_mutex_t *m) { - g_unlock_calls++; - g_lock_depth--; - return pthread_mutex_unlock(m); +int __wrap_pthread_mutex_unlock(pthread_mutex_t *m) { + g_unlock_calls++; + g_lock_depth--; + return __real_pthread_mutex_unlock(m); } -static void mock_pmem_free(void *p) { - g_free_calls++; - free(p); -} - -static void mock_psync_task_destroy(psync_task_manager_t tm) { - g_destroy_calls++; - g_lock_held_at_destroy = (g_lock_depth == 0); /* should be 0: unlocked before destroy */ - int i; - for (i = 0; i < tm->taskcnt; i++) - pthread_cond_destroy(&tm->tasks[i].cond); - pthread_mutex_destroy(&tm->mutex); - mock_pmem_free(tm); -} - -/* ------------------------------------------------------------------ */ -/* Replica of psync_task_free from the fixed branch */ -/* ------------------------------------------------------------------ */ -static void test_psync_task_free(psync_task_manager_t tm) { - int refcnt, i; - mock_mutex_lock(&tm->mutex); - if (tm->refcnt == 1) { - mock_mutex_unlock(&tm->mutex); - mock_psync_task_destroy(tm); - } else { - tm->waitfor = PSYNC_WAIT_FREED; - for (i = 0; i < tm->taskcnt; i++) - if (tm->tasks[i].status == PSYNC_TASK_STATUS_READY) { - tm->tasks[i].status = PSYNC_TASK_STATUS_RETURNED; - pthread_cond_signal(&tm->tasks[i].cond); - } - refcnt = --tm->refcnt; - mock_mutex_unlock(&tm->mutex); - if (!refcnt) - mock_psync_task_destroy(tm); - } +/* + * psync_task_destroy() calls pmem_free() as its last act. + * We intercept it to count destroy invocations and capture lock state. + * We call free() directly because make_tm() allocates with malloc(). + */ +void __wrap_pmem_free(pmem_subsystem_t subsystem, void *ptr) { + (void)subsystem; + g_destroy_calls++; + g_lock_held_at_destroy = (g_lock_depth == 0); /* should be 0 = unlocked */ + free(ptr); } /* ------------------------------------------------------------------ */ /* Helpers */ /* ------------------------------------------------------------------ */ + static int passes = 0, failures = 0; #define PASS(n) do { printf("PASS: %s\n", n); passes++; } while (0) #define FAIL(n, ...) do { printf("FAIL: %s — ", n); printf(__VA_ARGS__); printf("\n"); failures++; } while (0) static void reset(void) { - g_lock_calls = 0; - g_unlock_calls = 0; - g_destroy_calls = 0; - g_free_calls = 0; - g_lock_depth = 0; - g_lock_held_at_destroy = 0; + g_lock_calls = 0; + g_unlock_calls = 0; + g_destroy_calls = 0; + g_lock_depth = 0; + g_lock_held_at_destroy = 0; } -/* Allocate and initialize a task manager with `cnt` tasks */ +/* Allocate and initialise a task manager with `cnt` tasks, refcnt=`refcnt` */ static psync_task_manager_t make_tm(int cnt, int refcnt) { - size_t sz = sizeof(struct psync_task_manager_t_) + - cnt * sizeof(struct psync_task_t_); - psync_task_manager_t tm = malloc(sz); - memset(tm, 0, sz); - pthread_mutex_init(&tm->mutex, NULL); - tm->taskcnt = cnt; - tm->refcnt = refcnt; - tm->waitfor = PSYNC_WAIT_NOBODY; - int i; - for (i = 0; i < cnt; i++) { - pthread_cond_init(&tm->tasks[i].cond, NULL); - tm->tasks[i].id = i; - tm->tasks[i].status = PSYNC_TASK_STATUS_RUNNING; - } - return tm; + size_t sz = sizeof(struct psync_task_manager_t_) + + cnt * sizeof(struct psync_task_t_); + psync_task_manager_t tm = (psync_task_manager_t)malloc(sz); + memset(tm, 0, sz); + pthread_mutex_init(&tm->mutex, NULL); + tm->taskcnt = cnt; + tm->refcnt = refcnt; + tm->waitfor = PSYNC_WAIT_NOBODY; + int i; + for (i = 0; i < cnt; i++) { + pthread_cond_init(&tm->tasks[i].cond, NULL); + tm->tasks[i].id = i; + tm->tasks[i].status = PSYNC_TASK_STATUS_RUNNING; + } + return tm; } /* ------------------------------------------------------------------ */ -/* Tests */ +/* Tests — call the real psync_task_free() */ /* ------------------------------------------------------------------ */ -/* refcnt=1: destroy called, mutex unlocked before destroy */ +/* refcnt=1: destroy called once, mutex unlocked before destroy */ static void test_single_owner_free(void) { - reset(); - psync_task_manager_t tm = make_tm(2, 1); + reset(); + psync_task_manager_t tm = make_tm(2, 1); - test_psync_task_free(tm); /* tm is freed inside */ + psync_task_free(tm); /* tm freed inside via __wrap_pmem_free */ - if (g_destroy_calls != 1) - FAIL("single owner: destroy called once", "destroy_calls=%d", g_destroy_calls); - else if (!g_lock_held_at_destroy) - FAIL("single owner: mutex unlocked before destroy", "lock_depth was non-zero at destroy"); - else if (g_lock_calls != 1 || g_unlock_calls != 1) - FAIL("single owner: lock/unlock balanced", "lock=%d unlock=%d", g_lock_calls, g_unlock_calls); - else - PASS("single owner free: destroy called once, mutex unlocked before destroy"); + if (g_destroy_calls != 1) + FAIL("single owner: destroy called once", + "destroy_calls=%d", g_destroy_calls); + else if (!g_lock_held_at_destroy) + FAIL("single owner: mutex unlocked before destroy", + "lock_depth was non-zero at destroy"); + else if (g_lock_calls != 1 || g_unlock_calls != 1) + FAIL("single owner: lock/unlock balanced", + "lock=%d unlock=%d", g_lock_calls, g_unlock_calls); + else + PASS("single owner free: destroy called once, mutex unlocked before destroy"); } -/* refcnt=2, free last ref manually: destroy called after second decrement */ +/* refcnt already at 1 when we call free: same result as single-owner */ static void test_last_ref_destroys(void) { - reset(); - psync_task_manager_t tm = make_tm(1, 2); + reset(); + psync_task_manager_t tm = make_tm(1, 2); - /* Simulate first ref already released: lower refcnt to 1 without locking */ - tm->refcnt = 1; + /* Simulate the other ref already gone */ + tm->refcnt = 1; - test_psync_task_free(tm); /* this is now the last ref */ + psync_task_free(tm); - if (g_destroy_calls == 1 && g_lock_held_at_destroy) - PASS("last ref free (refcnt path 1): destroy called, mutex unlocked before destroy"); - else - FAIL("last ref free", "destroy_calls=%d lock_held_at_destroy=%d", - g_destroy_calls, g_lock_held_at_destroy); + if (g_destroy_calls == 1 && g_lock_held_at_destroy) + PASS("last ref free: destroy called, mutex unlocked before destroy"); + else + FAIL("last ref free", + "destroy_calls=%d lock_held_at_destroy=%d", + g_destroy_calls, g_lock_held_at_destroy); } /* refcnt=2, not last ref: refcnt decremented, destroy NOT called */ static void test_not_last_ref_no_destroy(void) { - reset(); - psync_task_manager_t tm = make_tm(1, 2); + reset(); + psync_task_manager_t tm = make_tm(1, 2); - test_psync_task_free(tm); + psync_task_free(tm); - if (g_destroy_calls != 0) - FAIL("not last ref: no destroy", "destroy_calls=%d", g_destroy_calls); - else if (tm->refcnt != 1) - FAIL("not last ref: refcnt decremented to 1", "refcnt=%d", tm->refcnt); - else - PASS("not last ref: no destroy, refcnt decremented to 1"); + if (g_destroy_calls != 0) + FAIL("not last ref: no destroy", + "destroy_calls=%d", g_destroy_calls); + else if (tm->refcnt != 1) + FAIL("not last ref: refcnt decremented to 1", + "refcnt=%d", tm->refcnt); + else + PASS("not last ref: no destroy, refcnt decremented to 1"); - /* Manual cleanup since we didn't destroy */ - pthread_cond_destroy(&tm->tasks[0].cond); - pthread_mutex_destroy(&tm->mutex); - free(tm); + /* Manual cleanup since psync_task_destroy was not called */ + pthread_cond_destroy(&tm->tasks[0].cond); + pthread_mutex_destroy(&tm->mutex); + free(tm); } -/* READY tasks get RETURNED status when freed with refcnt>1 */ +/* READY tasks get RETURNED status + cond signalled when freed with refcnt>1 */ static void test_ready_tasks_signaled(void) { - reset(); - psync_task_manager_t tm = make_tm(3, 2); - tm->tasks[0].status = PSYNC_TASK_STATUS_RUNNING; - tm->tasks[1].status = PSYNC_TASK_STATUS_READY; - tm->tasks[2].status = PSYNC_TASK_STATUS_DONE; + reset(); + psync_task_manager_t tm = make_tm(3, 2); + tm->tasks[0].status = PSYNC_TASK_STATUS_RUNNING; + tm->tasks[1].status = PSYNC_TASK_STATUS_READY; + tm->tasks[2].status = PSYNC_TASK_STATUS_DONE; - test_psync_task_free(tm); + psync_task_free(tm); - int ok = (tm->tasks[0].status == PSYNC_TASK_STATUS_RUNNING && - tm->tasks[1].status == PSYNC_TASK_STATUS_RETURNED && - tm->tasks[2].status == PSYNC_TASK_STATUS_DONE && - tm->waitfor == PSYNC_WAIT_FREED); + int ok = (tm->tasks[0].status == PSYNC_TASK_STATUS_RUNNING && + tm->tasks[1].status == PSYNC_TASK_STATUS_RETURNED && + tm->tasks[2].status == PSYNC_TASK_STATUS_DONE && + tm->waitfor == PSYNC_WAIT_FREED); - if (ok) - PASS("READY tasks signaled RETURNED, others unchanged, waitfor=FREED"); - else - FAIL("READY tasks signaled", - "statuses=[%d,%d,%d] waitfor=%d", - tm->tasks[0].status, tm->tasks[1].status, - tm->tasks[2].status, tm->waitfor); + if (ok) + PASS("READY tasks signaled RETURNED, others unchanged, waitfor=FREED"); + else + FAIL("READY tasks signaled", + "statuses=[%d,%d,%d] waitfor=%d", + tm->tasks[0].status, tm->tasks[1].status, + tm->tasks[2].status, tm->waitfor); - /* Cleanup */ - int i; - for (i = 0; i < 3; i++) pthread_cond_destroy(&tm->tasks[i].cond); - pthread_mutex_destroy(&tm->mutex); - free(tm); + /* Manual cleanup */ + int i; + for (i = 0; i < 3; i++) pthread_cond_destroy(&tm->tasks[i].cond); + pthread_mutex_destroy(&tm->mutex); + free(tm); } -/* Lock is acquired before refcnt is read (core fix) */ +/* Mutex acquired before refcnt is inspected (core fix) */ static void test_lock_before_refcnt_check(void) { - reset(); - psync_task_manager_t tm = make_tm(1, 1); + reset(); + psync_task_manager_t tm = make_tm(1, 1); - /* We can only verify indirectly: lock_calls >= 1 before destroy fires. - * g_lock_held_at_destroy==1 means lock was acquired and released before destroy. */ - test_psync_task_free(tm); + psync_task_free(tm); - if (g_lock_calls >= 1 && g_lock_held_at_destroy) - PASS("mutex acquired before refcnt check; unlocked cleanly before destroy"); - else - FAIL("lock before refcnt check", - "lock_calls=%d lock_held_at_destroy=%d", g_lock_calls, g_lock_held_at_destroy); + /* g_lock_calls >= 1 means lock was acquired; g_lock_held_at_destroy = 1 + * means it was released cleanly before destroy fired */ + if (g_lock_calls >= 1 && g_lock_held_at_destroy) + PASS("mutex acquired before refcnt check; unlocked cleanly before destroy"); + else + FAIL("lock before refcnt check", + "lock_calls=%d lock_held_at_destroy=%d", + g_lock_calls, g_lock_held_at_destroy); } /* ------------------------------------------------------------------ */ int main(void) { - test_single_owner_free(); - test_last_ref_destroys(); - test_not_last_ref_no_destroy(); - test_ready_tasks_signaled(); - test_lock_before_refcnt_check(); + test_single_owner_free(); + test_last_ref_destroys(); + test_not_last_ref_no_destroy(); + test_ready_tasks_signaled(); + test_lock_before_refcnt_check(); - printf("\n%d passed, %d failed\n", passes, failures); - return failures ? 1 : 0; + printf("\n%d passed, %d failed\n", passes, failures); + return failures ? 1 : 0; }