Fix debug build: compile error, false-positive abort, crash DB lock (#372)

* Fix debug build: compile error, false-positive abort, and crash DB lock

- psql_debug.c: add forward declaration for psql_do_prepare to fix
  conflicting-types compile error (BUILD=debug was broken entirely)

- pfs_debug.c: change pfs_debug_check_lock_order from abort to log-only;
  write paths legitimately take file lock before SQL and handle ordering
  via psql_trylock()+relock in pfs_reopen_file_for_writing — no actual
  deadlock risk, the check was a false positive

- psignal.c/h: add psignal_register_cleanup() hook mechanism; change
  panic() to use _exit(1) instead of abort() so all file descriptors are
  closed on crash, releasing SQLite WAL POSIX advisory locks immediately
  and preventing ASan from hanging the process as a zombie

- psql.c: register psql_panic_cleanup() hook to close the DB on panic
  (belt-and-suspenders alongside _exit fd cleanup)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix debug psql_trylock: missing strong override left lockctr unupdated

The weak psql_trylock() stub in psql.c called plocks_trywrlock() directly
without updating lockctr. In the debug build, psql_unlock() asserts
lockctr > 0, so when trylock succeeded (lock acquired, lockctr still 0)
the assert fired with SIGABRT on write ops via pfs_inc_writeid_locked.

Add a strong psql_trylock() override in psql_debug.c that delegates to
psql_do_trylock(), which properly acquires the rwlock and updates lockctr.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Levi Neely 2026-03-09 20:49:09 +01:00 committed by GitHub
parent 358ae595e9
commit 90945b8817
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 43 additions and 6 deletions

View File

@ -47,8 +47,11 @@ void pfs_debug_register_signal_handlers() {
}
void pfs_debug_check_lock_order(const char *file, unsigned long line) {
if (!psql_locked()) {
pdbg_logf(D_ERROR, "lock ordering violation: pfs_lock_file called without psql_lock at %s:%lu", file, line);
abort();
}
/* Lock order is sql -> file, but write paths legitimately take file lock
* first and handle sql acquisition via psql_trylock()+relock in
* pfs_reopen_file_for_writing. No actual deadlock risk; just log. */
if (!psql_locked())
pdbg_logf(D_NOTICE,
"pfs_lock_file called without psql_lock at %s:%lu (expected for write path)",
file, line);
}

View File

@ -39,6 +39,10 @@
extern PSYNC_THREAD const char *psync_thread_name;
/* Forward declaration */
psync_sql_res *psql_do_prepare(const char *sql, const char *file,
unsigned line);
// --------------------------------------------------------------------------
// Debug-only state
// --------------------------------------------------------------------------
@ -223,6 +227,10 @@ void psql_do_rdlock(const char *file, unsigned line) {
// Strong overrides for same-named functions (weak in psql.c)
// --------------------------------------------------------------------------
int psql_trylock() {
return psql_do_trylock(__FILE__, __LINE__);
}
void psql_lock() {
psql_do_lock(__FILE__, __LINE__);
}

View File

@ -4,6 +4,16 @@
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#define PSIGNAL_MAX_CLEANUPS 16
static void (*cleanup_fns[PSIGNAL_MAX_CLEANUPS])(void);
static int cleanup_count = 0;
void psignal_register_cleanup(void (*fn)(void)) {
if (cleanup_count < PSIGNAL_MAX_CLEANUPS)
cleanup_fns[cleanup_count++] = fn;
}
static volatile sig_atomic_t sigint_flag = 0;
static volatile sig_atomic_t sigterm_flag = 0;
@ -42,8 +52,15 @@ void panic(const char *msg) {
signal(SIGSEGV, SIG_DFL);
signal(SIGABRT, SIG_DFL);
signal(SIGBUS, SIG_DFL);
abort();
for (int i = 0; i < cleanup_count; i++)
cleanup_fns[i]();
/* Use _exit() instead of abort(): _exit closes all file descriptors
* immediately, releasing POSIX advisory locks (including SQLite's WAL
* locks). abort() is intercepted by ASan which can hang indefinitely,
* leaving the process as a zombie and the DB locked. */
_exit(1);
}
static void panic_handler(int sig) {

View File

@ -10,6 +10,7 @@ extern "C" {
void psignal_register(int signum);
int psignal_check_pending(void);
void psignal_set_custom_handler(int sig, void (*handler)(int));
void psignal_register_cleanup(void (*fn)(void));
void panic(const char *msg) __attribute__((noreturn));
#ifdef __cplusplus

View File

@ -12,6 +12,7 @@
#include "psettings.h"
#include "psql.h"
#include "psql_internal.h"
#include "psignal.h"
#include "psys.h"
// psql.h defines function-like macros (e.g. psql_lock() -> psql_do_lock())
@ -82,6 +83,12 @@ static void on_error(void *ptr, int code, const char *msg) {
pdbg_logf(D_WARNING, "database warning %d: %s", code, msg);
}
static void psql_panic_cleanup(void) {
if (psync_db)
sqlite3_close_v2(psync_db);
psync_db = NULL;
}
static void psync_sql_free_cache(void *ptr) {
psync_sql_res *res = (psync_sql_res *)ptr;
sqlite3_finalize(res->stmt);
@ -199,6 +206,7 @@ int psql_connect(const char *db) {
pthread_mutex_init(&cpmutex, &mattr);
pthread_mutexattr_destroy(&mattr);
initmutex = 0;
psignal_register_cleanup(psql_panic_cleanup);
}
if (IS_DEBUG) {
sqlite3_config(SQLITE_CONFIG_LOG, on_error, NULL);