Skip to content

Commit 59e1077

Browse files
andinuxclaude
andcommitted
fix: avoid crash when reopening db with stale cloudsync_table_settings
Dropping a table and its <table>_cloudsync meta-table without calling cloudsync_cleanup left stale rows in cloudsync_table_settings. On the next connection, loading the extension crashed with a double-free. Two bugs: - cloudsync_dbversion_rebuild returned DBRES_NOMEM when the build query yielded NULL (no *_cloudsync tables in sqlite_master), failing init. Treat NULL the same as count == 0: no prepared stmt, rebuilt later. - On init failure, dbsync_register_functions manually freed the context that SQLite already owns via the cloudsync_version destructor, causing a double-free on sqlite3_close. Let SQLite release it instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 52224a3 commit 59e1077

3 files changed

Lines changed: 90 additions & 3 deletions

File tree

src/cloudsync.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,14 @@ int cloudsync_dbversion_rebuild (cloudsync_context *data) {
397397
int64_t count = dbutils_table_settings_count_tables(data);
398398
if (count == 0) return DBRES_OK;
399399
else if (count == -1) return cloudsync_set_dberror(data);
400-
400+
401401
char *sql = cloudsync_dbversion_build_query(data);
402-
if (!sql) return DBRES_NOMEM;
402+
// A NULL result here means sqlite_master has no *_cloudsync meta-tables
403+
// (for example, the user dropped the base table and its meta-table without
404+
// calling cloudsync_cleanup, leaving stale cloudsync_table_settings rows).
405+
// Treat this the same as count == 0: no prepared statement, db_version
406+
// stays at the minimum and will be rebuilt on the next cloudsync_init.
407+
if (!sql) return DBRES_OK;
403408
DEBUG_SQL("db_version_stmt: %s", sql);
404409

405410
int rc = databasevm_prepare(data, sql, (void **)&data->db_version_stmt, DBFLAG_PERSISTENT);

src/sqlite/cloudsync_sqlite.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1470,7 +1470,10 @@ int dbsync_register_functions (sqlite3 *db, char **pzErrMsg) {
14701470
// load config, if exists
14711471
if (cloudsync_config_exists(data)) {
14721472
if (cloudsync_context_init(data) == NULL) {
1473-
cloudsync_context_free(data);
1473+
// Do not free ctx here: it is already owned by the cloudsync_version
1474+
// function (registered above with cloudsync_context_free as its
1475+
// destructor). SQLite will release it when the connection is closed.
1476+
// Freeing it manually would cause a double-free on sqlite3_close.
14741477
if (pzErrMsg) *pzErrMsg = sqlite3_mprintf("An error occurred while trying to initialize context");
14751478
return SQLITE_ERROR;
14761479
}

test/unit.c

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2401,6 +2401,84 @@ bool do_test_stale_table_settings(bool cleanup_databases) {
24012401
return result;
24022402
}
24032403

2404+
// Same as do_test_stale_table_settings, but also drops the <table>_cloudsync
2405+
// meta-table before reopening. With a stale cloudsync_table_settings row and
2406+
// no matching *_cloudsync meta-table in sqlite_master, the dbversion query
2407+
// builder produces an empty (NULL) SQL string, causing sqlite3_cloudsync_init
2408+
// to fail on reopen — previously crashing in some environments.
2409+
bool do_test_stale_table_settings_dropped_meta(bool cleanup_databases) {
2410+
bool result = false;
2411+
char dbpath[256];
2412+
time_t timestamp = time(NULL);
2413+
2414+
#ifdef __ANDROID__
2415+
snprintf(dbpath, sizeof(dbpath), "%s/cloudsync-test-stale-meta-%ld.sqlite", ".", timestamp);
2416+
#else
2417+
snprintf(dbpath, sizeof(dbpath), "%s/cloudsync-test-stale-meta-%ld.sqlite", getenv("HOME"), timestamp);
2418+
#endif
2419+
2420+
// Phase 1: create database, table, and init cloudsync
2421+
sqlite3 *db = NULL;
2422+
int rc = sqlite3_open(dbpath, &db);
2423+
if (rc != SQLITE_OK) return false;
2424+
sqlite3_exec(db, "PRAGMA journal_mode=WAL;", NULL, NULL, NULL);
2425+
sqlite3_cloudsync_init(db, NULL, NULL);
2426+
2427+
rc = sqlite3_exec(db, "CREATE TABLE cloud (id TEXT PRIMARY KEY NOT NULL, value TEXT, extra INTEGER);", NULL, NULL, NULL);
2428+
if (rc != SQLITE_OK) goto cleanup;
2429+
2430+
rc = sqlite3_exec(db, "SELECT cloudsync_init('cloud');", NULL, NULL, NULL);
2431+
if (rc != SQLITE_OK) goto cleanup;
2432+
2433+
// Phase 2: drop both the base table AND the meta-table without calling
2434+
// cloudsync_cleanup. This leaves stale entries in cloudsync_table_settings
2435+
// with no matching *_cloudsync table in sqlite_master.
2436+
sqlite3_exec(db, "SELECT cloudsync_terminate();", NULL, NULL, NULL);
2437+
sqlite3_close(db);
2438+
db = NULL;
2439+
2440+
rc = sqlite3_open(dbpath, &db);
2441+
if (rc != SQLITE_OK) goto cleanup;
2442+
rc = sqlite3_exec(db, "DROP TABLE IF EXISTS cloud;", NULL, NULL, NULL);
2443+
if (rc != SQLITE_OK) goto cleanup;
2444+
rc = sqlite3_exec(db, "DROP TABLE IF EXISTS cloud_cloudsync;", NULL, NULL, NULL);
2445+
if (rc != SQLITE_OK) goto cleanup;
2446+
sqlite3_close(db);
2447+
db = NULL;
2448+
2449+
// Phase 3: reopen the database and load the extension — must succeed.
2450+
rc = sqlite3_open(dbpath, &db);
2451+
if (rc != SQLITE_OK) goto cleanup;
2452+
rc = sqlite3_cloudsync_init(db, NULL, NULL);
2453+
if (rc != SQLITE_OK) goto cleanup;
2454+
2455+
// Sanity check: we can still call cloudsync_version and create a new table.
2456+
rc = sqlite3_exec(db, "SELECT cloudsync_version();", NULL, NULL, NULL);
2457+
if (rc != SQLITE_OK) goto cleanup;
2458+
2459+
rc = sqlite3_exec(db, "CREATE TABLE cloud2 (id TEXT PRIMARY KEY NOT NULL, v TEXT);", NULL, NULL, NULL);
2460+
if (rc != SQLITE_OK) goto cleanup;
2461+
rc = sqlite3_exec(db, "SELECT cloudsync_init('cloud2');", NULL, NULL, NULL);
2462+
if (rc != SQLITE_OK) goto cleanup;
2463+
2464+
result = true;
2465+
2466+
cleanup:
2467+
if (db) {
2468+
sqlite3_exec(db, "SELECT cloudsync_terminate();", NULL, NULL, NULL);
2469+
sqlite3_close(db);
2470+
}
2471+
if (cleanup_databases) {
2472+
file_delete_internal(dbpath);
2473+
char walpath[280];
2474+
snprintf(walpath, sizeof(walpath), "%s-wal", dbpath);
2475+
file_delete_internal(walpath);
2476+
snprintf(walpath, sizeof(walpath), "%s-shm", dbpath);
2477+
file_delete_internal(walpath);
2478+
}
2479+
return result;
2480+
}
2481+
24042482
// Authorizer state for do_test_context_cb_error_cleanup.
24052483
// Denies INSERT on a specific table after allowing a set number of INSERTs.
24062484
static const char *g_deny_insert_table = NULL;
@@ -12268,6 +12346,7 @@ int main (int argc, const char * argv[]) {
1226812346
result += test_report("Large Composite PK Test:", do_test_large_composite_pk(2, print_result, cleanup_databases));
1226912347
result += test_report("Schema Hash Mismatch:", do_test_schema_hash_mismatch(2, print_result, cleanup_databases));
1227012348
result += test_report("Stale Table Settings:", do_test_stale_table_settings(cleanup_databases));
12349+
result += test_report("Stale Table Settings Dropped Meta:", do_test_stale_table_settings_dropped_meta(cleanup_databases));
1227112350
result += test_report("Block LWW Existing Data:", do_test_block_lww_existing_data(cleanup_databases));
1227212351
result += test_report("Block Column Reload:", do_test_block_column_reload(cleanup_databases));
1227312352
result += test_report("CB Error Cleanup:", do_test_context_cb_error_cleanup());

0 commit comments

Comments
 (0)