Skip to content

Commit 60d5893

Browse files
andinuxclaude
andauthored
fix: stale cloudsync_table_settings crash on reopen (#40)
Reopening a database whose base table and <table>_cloudsync meta-table were dropped without calling cloudsync_cleanup crashed with a double-free on sqlite3_close (SQLite) and ereport'd "An error occurred while trying to initialize context" on every subsequent cloudsync call (PostgreSQL). Two independent bugs were involved: 1. src/cloudsync.c — cloudsync_dbversion_rebuild returned DBRES_NOMEM when cloudsync_dbversion_build_query yielded a NULL SQL string. That NULL happens benignly when cloudsync_table_settings still has a row but sqlite_master / pg_tables no longer has any matching *_cloudsync table. The build helper now returns rc via an out-param so the caller can distinguish "no meta-tables" (rc == OK && sql == NULL, treated like count == 0 — no prepared statement, db_version stays at the minimum and is rebuilt on the next cloudsync_init) from a real database_select_text failure, which is now propagated via cloudsync_set_dberror instead of silently leaving db_version_stmt unset (where later writes would fall back to CLOUDSYNC_MIN_DB_VERSION and emit duplicate/rewound db_version values). 2. src/sqlite/cloudsync_sqlite.c — on the init-failure branch, dbsync_register_functions manually called cloudsync_context_free on a context that SQLite already owned via the cloudsync_version function's destructor. sqlite3_close then freed it a second time. Removed the manual free; SQLite cleans up on close. Version bumped to 1.0.14 and CHANGELOG updated. Tests: - test/unit.c do_test_stale_table_settings_dropped_meta — reproduces the original double-free crash via drop base + drop meta + reopen. - test/unit.c do_test_dbversion_rebuild_error — installs an sqlite3_set_authorizer denying reads of sqlite_master so SQL_DBVERSION_BUILD_QUERY fails with SQLITE_AUTH, asserting that cloudsync_dbversion_rebuild now surfaces the error (non-OK rc plus cloudsync_errcode/errmsg set). - test/postgresql/51_stale_table_settings_dropped_meta.sql — PostgreSQL equivalent of the reopen scenario; verified to reproduce "An error occurred while trying to initialize context" without the shared-code fix and pass with it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 52224a3 commit 60d5893

7 files changed

Lines changed: 318 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file.
44

55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
66

7+
## [1.0.14] - 2026-04-15
8+
9+
### Fixed
10+
11+
- **Stale `cloudsync_table_settings` crash**: Reopening a database that had its base table and `<table>_cloudsync` meta-table dropped without calling `cloudsync_cleanup` crashed with a double-free on `sqlite3_close`. Two bugs were involved: (1) `cloudsync_dbversion_rebuild` returned `DBRES_NOMEM` when `cloudsync_dbversion_build_query` yielded a NULL SQL string (stale row in `cloudsync_table_settings` but no matching `*_cloudsync` table in `sqlite_master`), failing extension init; (2) on init failure `dbsync_register_functions` manually freed the context that SQLite already owned via the `cloudsync_version` destructor, causing a double-free when the connection was later closed. `cloudsync_dbversion_rebuild` now treats a NULL build query the same as `count == 0` (no prepared statement, db_version stays at the minimum and is rebuilt on the next `cloudsync_init`), and the manual free in the error path has been removed.
12+
13+
### Added
14+
15+
- Unit test `do_test_stale_table_settings_dropped_meta` (Stale Table Settings Dropped Meta) covering the drop-base-table + drop-meta-table + reopen scenario.
16+
717
## [1.0.13] - 2026-04-14
818

919
### Fixed

src/cloudsync.c

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,11 @@ void dbvm_reset (dbvm_t *stmt) {
363363

364364
// MARK: - Database Version -
365365

366-
char *cloudsync_dbversion_build_query (cloudsync_context *data) {
366+
int cloudsync_dbversion_build_query (cloudsync_context *data, char **sql_out) {
367367
// this function must be manually called each time tables changes
368368
// because the query plan changes too and it must be re-prepared
369369
// unfortunately there is no other way
370-
370+
371371
// we need to execute a query like:
372372
/*
373373
SELECT max(version) as version FROM (
@@ -380,29 +380,38 @@ char *cloudsync_dbversion_build_query (cloudsync_context *data) {
380380
SELECT value as version FROM cloudsync_settings WHERE key = 'pre_alter_dbversion'
381381
)
382382
*/
383-
383+
384384
// the good news is that the query can be computed in SQLite without the need to do any extra computation from the host language
385-
386-
char *value = NULL;
387-
int rc = database_select_text(data, SQL_DBVERSION_BUILD_QUERY, &value);
388-
return (rc == DBRES_OK) ? value : NULL;
385+
386+
*sql_out = NULL;
387+
return database_select_text(data, SQL_DBVERSION_BUILD_QUERY, sql_out);
389388
}
390389

391390
int cloudsync_dbversion_rebuild (cloudsync_context *data) {
392391
if (data->db_version_stmt) {
393392
databasevm_finalize(data->db_version_stmt);
394393
data->db_version_stmt = NULL;
395394
}
396-
395+
397396
int64_t count = dbutils_table_settings_count_tables(data);
398397
if (count == 0) return DBRES_OK;
399398
else if (count == -1) return cloudsync_set_dberror(data);
400-
401-
char *sql = cloudsync_dbversion_build_query(data);
402-
if (!sql) return DBRES_NOMEM;
399+
400+
char *sql = NULL;
401+
int rc = cloudsync_dbversion_build_query(data, &sql);
402+
if (rc != DBRES_OK) return cloudsync_set_dberror(data);
403+
404+
// A NULL SQL with rc == OK means the generator produced a NULL row:
405+
// sqlite_master has no *_cloudsync meta-tables (for example, the user
406+
// dropped the base table and its meta-table without calling
407+
// cloudsync_cleanup, leaving stale cloudsync_table_settings rows).
408+
// Treat this the same as count == 0: no prepared statement, db_version
409+
// stays at the minimum and will be rebuilt on the next cloudsync_init.
410+
// Genuine errors from database_select_text are handled above.
411+
if (!sql) return DBRES_OK;
403412
DEBUG_SQL("db_version_stmt: %s", sql);
404-
405-
int rc = databasevm_prepare(data, sql, (void **)&data->db_version_stmt, DBFLAG_PERSISTENT);
413+
414+
rc = databasevm_prepare(data, sql, (void **)&data->db_version_stmt, DBFLAG_PERSISTENT);
406415
DEBUG_STMT("db_version_stmt %p", data->db_version_stmt);
407416
cloudsync_memory_free(sql);
408417
return rc;

src/cloudsync.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
extern "C" {
1919
#endif
2020

21-
#define CLOUDSYNC_VERSION "1.0.13"
21+
#define CLOUDSYNC_VERSION "1.0.14"
2222
#define CLOUDSYNC_MAX_TABLENAME_LEN 512
2323

2424
#define CLOUDSYNC_VALUE_NOTSET -1

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
}
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
-- 'Stale cloudsync_table_settings with dropped meta-table'
2+
-- Mirrors the SQLite unit test: Stale Table Settings Dropped Meta
3+
--
4+
-- When a user drops a tracked table and its <table>_cloudsync meta-table
5+
-- manually (without calling cloudsync_cleanup), cloudsync_table_settings is
6+
-- left with stale rows while pg_tables no longer has any matching
7+
-- *_cloudsync table. Before the fix in cloudsync_dbversion_rebuild, opening a
8+
-- new backend and calling any cloudsync function caused
9+
-- cloudsync_dbversion_build_query to produce a NULL SQL string (string_agg
10+
-- over zero rows), which was misreported as DBRES_NOMEM, making
11+
-- cloudsync_context_init fail and every cloudsync_* call ereport ERROR.
12+
13+
\set testid '51'
14+
\ir helper_test_init.sql
15+
16+
\connect postgres
17+
\ir helper_psql_conn_setup.sql
18+
19+
DROP DATABASE IF EXISTS cloudsync_test_51;
20+
CREATE DATABASE cloudsync_test_51;
21+
22+
\connect cloudsync_test_51
23+
\ir helper_psql_conn_setup.sql
24+
25+
CREATE EXTENSION IF NOT EXISTS cloudsync;
26+
27+
-- Phase 1: create a tracked table and initialize cloudsync on it.
28+
DROP TABLE IF EXISTS stale_doc CASCADE;
29+
CREATE TABLE stale_doc (id TEXT PRIMARY KEY NOT NULL, body TEXT);
30+
SELECT cloudsync_init('stale_doc', 'CLS', 1) AS _init \gset
31+
32+
-- Sanity: the meta-table exists and cloudsync_table_settings has a row for it.
33+
SELECT count(*) AS meta_exists FROM pg_tables WHERE tablename = 'stale_doc_cloudsync' \gset
34+
SELECT (:meta_exists::int = 1) AS meta_exists_ok \gset
35+
\if :meta_exists_ok
36+
\echo [PASS] (:testid) stale_doc_cloudsync meta-table created
37+
\else
38+
\echo [FAIL] (:testid) expected stale_doc_cloudsync to exist
39+
SELECT (:fail::int + 1) AS fail \gset
40+
\endif
41+
42+
SELECT count(*) AS settings_rows FROM cloudsync_table_settings WHERE tbl_name = 'stale_doc' \gset
43+
SELECT (:settings_rows::int > 0) AS settings_rows_ok \gset
44+
\if :settings_rows_ok
45+
\echo [PASS] (:testid) cloudsync_table_settings has row(s) for stale_doc
46+
\else
47+
\echo [FAIL] (:testid) expected cloudsync_table_settings row for stale_doc
48+
SELECT (:fail::int + 1) AS fail \gset
49+
\endif
50+
51+
-- Phase 2: drop BOTH the base table and the meta-table without calling
52+
-- cloudsync_cleanup. cloudsync_table_settings still references stale_doc,
53+
-- but pg_tables has no *_cloudsync tables at all now.
54+
DROP TABLE stale_doc;
55+
DROP TABLE stale_doc_cloudsync;
56+
57+
SELECT count(*) AS cloudsync_meta_tables FROM pg_tables WHERE tablename LIKE '%_cloudsync' \gset
58+
SELECT (:cloudsync_meta_tables::int = 0) AS no_meta_ok \gset
59+
\if :no_meta_ok
60+
\echo [PASS] (:testid) no *_cloudsync meta-tables remain in pg_tables
61+
\else
62+
\echo [FAIL] (:testid) expected zero *_cloudsync tables, got :cloudsync_meta_tables
63+
SELECT (:fail::int + 1) AS fail \gset
64+
\endif
65+
66+
-- Phase 3: reconnect to force a fresh backend. pg_cloudsync_context is a
67+
-- static per-process pointer, so a new backend means
68+
-- cloudsync_pg_context_init runs again on the next cloudsync call — which
69+
-- is exactly what used to fail under this bug.
70+
\connect cloudsync_test_51
71+
\ir helper_psql_conn_setup.sql
72+
73+
-- cloudsync_version is a pure function that does not touch the context, so
74+
-- this call cannot fail even with the bug present. It's here only as a
75+
-- trivial smoke check that the extension is still loadable.
76+
SELECT cloudsync_version() IS NOT NULL AS version_ok \gset
77+
\if :version_ok
78+
\echo [PASS] (:testid) cloudsync_version() reachable after reopen
79+
\else
80+
\echo [FAIL] (:testid) cloudsync_version() failed after reopen
81+
SELECT (:fail::int + 1) AS fail \gset
82+
\endif
83+
84+
-- The real check: calling a function that goes through get_cloudsync_context()
85+
-- must succeed. Before the fix, cloudsync_dbversion_rebuild returned
86+
-- DBRES_NOMEM here because SQL_DBVERSION_BUILD_QUERY's string_agg over zero
87+
-- rows produced a NULL SQL string, and the whole init path would ereport
88+
-- ERROR — any cloudsync_* call below would abort the script.
89+
CREATE TABLE stale_doc2 (id TEXT PRIMARY KEY NOT NULL, body TEXT);
90+
SELECT cloudsync_init('stale_doc2', 'CLS', 1) AS _init2 \gset
91+
92+
-- The new table's meta-table exists. If cloudsync_init failed (pre-fix
93+
-- behavior) this count will be 0, covering both the init-rc check and the
94+
-- meta-table creation in a single assertion.
95+
SELECT count(*) AS meta2_exists FROM pg_tables WHERE tablename = 'stale_doc2_cloudsync' \gset
96+
SELECT (:meta2_exists::int = 1) AS meta2_exists_ok \gset
97+
\if :meta2_exists_ok
98+
\echo [PASS] (:testid) cloudsync_init succeeded and stale_doc2_cloudsync was created
99+
\else
100+
\echo [FAIL] (:testid) expected stale_doc2_cloudsync to exist after cloudsync_init
101+
SELECT (:fail::int + 1) AS fail \gset
102+
\endif
103+
104+
-- An insert via the new cloudsync_init'd table should produce a cloudsync
105+
-- metadata entry — confirming the context is fully functional.
106+
INSERT INTO stale_doc2 (id, body) VALUES ('a', 'hello');
107+
108+
SELECT count(*) AS meta_rows FROM stale_doc2_cloudsync \gset
109+
SELECT (:meta_rows::int > 0) AS meta_rows_ok \gset
110+
\if :meta_rows_ok
111+
\echo [PASS] (:testid) stale_doc2_cloudsync has metadata after insert
112+
\else
113+
\echo [FAIL] (:testid) expected metadata rows in stale_doc2_cloudsync
114+
SELECT (:fail::int + 1) AS fail \gset
115+
\endif
116+
117+
-- Cleanup
118+
\ir helper_test_cleanup.sql
119+
\if :should_cleanup
120+
DROP DATABASE IF EXISTS cloudsync_test_51;
121+
\else
122+
\echo [INFO] !!!!!
123+
\endif

test/postgresql/full_test.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
\ir 48_row_filter_multi_table.sql
5959
\ir 49_row_filter_prefill.sql
6060
\ir 50_block_lww_existing_data.sql
61+
\ir 51_stale_table_settings_dropped_meta.sql
6162

6263
-- 'Test summary'
6364
\echo '\nTest summary:'

0 commit comments

Comments
 (0)