Skip to content

Commit c97f7da

Browse files
andinuxclaude
andcommitted
fix: prevent infinite loop when reloading persisted block-column settings
When a database with a persisted block-column setting was reopened, sqlite3_cloudsync_init looped forever inside dbutils_settings_table_load_callback. The callback iterates cloudsync_table_settings via sqlite3_exec and, for each algo=block row, called cloudsync_setup_block_column, which unconditionally REPLACEd the same row back into the table. The rewritten row was re-fed to the still-open cursor, triggering the callback again ad infinitum. Add a `persist` flag to cloudsync_setup_block_column so the settings loader can replay the in-memory setup without writing back to cloudsync_table_settings. The SQL-function callers (cloudsync_set_column in both the SQLite and PostgreSQL backends) keep persisting by passing true. Add a regression test (Block Column Reload) that creates a block column with a custom delimiter, closes the database, and reopens it — without the fix this hangs the test process. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ec7091a commit c97f7da

6 files changed

Lines changed: 107 additions & 12 deletions

File tree

src/cloudsync.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,7 +2069,7 @@ int merge_insert (cloudsync_context *data, cloudsync_table_context *table, const
20692069

20702070
// MARK: - Block column setup -
20712071

2072-
int cloudsync_setup_block_column (cloudsync_context *data, const char *table_name, const char *col_name, const char *delimiter) {
2072+
int cloudsync_setup_block_column (cloudsync_context *data, const char *table_name, const char *col_name, const char *delimiter, bool persist) {
20732073
cloudsync_table_context *table = table_lookup(data, table_name);
20742074
if (!table) return cloudsync_set_error(data, "cloudsync_setup_block_column: table not found", DBRES_ERROR);
20752075

@@ -2137,13 +2137,17 @@ int cloudsync_setup_block_column (cloudsync_context *data, const char *table_nam
21372137
if (rc != DBRES_OK) return rc;
21382138
}
21392139

2140-
// Persist settings
2141-
int rc = dbutils_table_settings_set_key_value(data, table_name, col_name, "algo", "block");
2142-
if (rc != DBRES_OK) return rc;
2143-
2144-
if (delimiter) {
2145-
rc = dbutils_table_settings_set_key_value(data, table_name, col_name, "delimiter", delimiter);
2140+
// Persist settings (skipped when called from the settings loader, since
2141+
// writing to cloudsync_table_settings while sqlite3_exec is iterating it
2142+
// re-feeds the rewritten row to the cursor and causes an infinite loop).
2143+
if (persist) {
2144+
int rc = dbutils_table_settings_set_key_value(data, table_name, col_name, "algo", "block");
21462145
if (rc != DBRES_OK) return rc;
2146+
2147+
if (delimiter) {
2148+
rc = dbutils_table_settings_set_key_value(data, table_name, col_name, "delimiter", delimiter);
2149+
if (rc != DBRES_OK) return rc;
2150+
}
21472151
}
21482152

21492153
return DBRES_OK;

src/cloudsync.h

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

21-
#define CLOUDSYNC_VERSION "1.0.11"
21+
#define CLOUDSYNC_VERSION "1.0.12"
2222
#define CLOUDSYNC_MAX_TABLENAME_LEN 512
2323

2424
#define CLOUDSYNC_VALUE_NOTSET -1
@@ -119,7 +119,7 @@ col_algo_t table_col_algo (cloudsync_table_context *table, int index);
119119
const char *table_col_delimiter (cloudsync_table_context *table, int index);
120120
int table_col_index (cloudsync_table_context *table, const char *col_name);
121121
int block_materialize_column (cloudsync_context *data, cloudsync_table_context *table, const void *pk, int pklen, const char *base_col_name);
122-
int cloudsync_setup_block_column (cloudsync_context *data, const char *table_name, const char *col_name, const char *delimiter);
122+
int cloudsync_setup_block_column (cloudsync_context *data, const char *table_name, const char *col_name, const char *delimiter, bool persist);
123123

124124
// Block column accessors (avoids accessing opaque struct from outside cloudsync.c)
125125
dbvm_t *table_block_value_read_stmt (cloudsync_table_context *table);

src/dbutils.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ int dbutils_settings_table_load_callback (void *xdata, int ncols, char **values,
385385
char dbuf[256];
386386
int drc = dbutils_table_settings_get_value(data, table_name, col_name, "delimiter", dbuf, sizeof(dbuf));
387387
const char *delim = (drc == DBRES_OK && dbuf[0]) ? dbuf : NULL;
388-
cloudsync_setup_block_column(data, table_name, col_name, delim);
388+
cloudsync_setup_block_column(data, table_name, col_name, delim, false);
389389
DEBUG_SETTINGS("load block column: %s.%s delimiter: %s", table_name, col_name, delim ? delim : "(default)");
390390
continue;
391391
}

src/postgresql/cloudsync_postgresql.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ Datum cloudsync_set_column (PG_FUNCTION_ARGS) {
617617
{
618618
// Handle block column setup: cloudsync_set_column('tbl', 'col', 'algo', 'block')
619619
if (key && value && strcmp(key, "algo") == 0 && strcmp(value, "block") == 0) {
620-
int rc = cloudsync_setup_block_column(data, tbl, col, NULL);
620+
int rc = cloudsync_setup_block_column(data, tbl, col, NULL, true);
621621
if (rc != DBRES_OK) {
622622
ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("%s", cloudsync_errmsg(data))));
623623
}

src/sqlite/cloudsync_sqlite.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ void dbsync_set_column (sqlite3_context *context, int argc, sqlite3_value **argv
150150

151151
// Handle block column setup: cloudsync_set_column('tbl', 'col', 'algo', 'block')
152152
if (key && value && strcmp(key, "algo") == 0 && strcmp(value, "block") == 0) {
153-
int rc = cloudsync_setup_block_column(data, tbl, col, NULL);
153+
int rc = cloudsync_setup_block_column(data, tbl, col, NULL, true);
154154
if (rc != DBRES_OK) {
155155
sqlite3_result_error(context, cloudsync_errmsg(data), -1);
156156
}

test/unit.c

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9001,6 +9001,96 @@ static int64_t do_select_int(sqlite3 *db, const char *sql) {
90019001
return val;
90029002
}
90039003

9004+
// Regression: persisted block-column settings must not cause an infinite loop
9005+
// when the extension is reloaded. Without the fix, dbutils_settings_table_load_callback
9006+
// (driven by sqlite3_exec over cloudsync_table_settings) would re-invoke
9007+
// cloudsync_setup_block_column, which REPLACEd the same row mid-iteration and
9008+
// re-fed it back into the cursor.
9009+
bool do_test_block_column_reload(bool cleanup_databases) {
9010+
bool result = false;
9011+
char dbpath[256];
9012+
time_t timestamp = time(NULL);
9013+
9014+
#ifdef __ANDROID__
9015+
snprintf(dbpath, sizeof(dbpath), "%s/cloudsync-test-blockreload-%ld.sqlite", ".", timestamp);
9016+
#else
9017+
snprintf(dbpath, sizeof(dbpath), "%s/cloudsync-test-blockreload-%ld.sqlite", getenv("HOME"), timestamp);
9018+
#endif
9019+
9020+
// Phase 1: create database, table, init cloudsync, mark a column as block algo.
9021+
// Use a custom delimiter so both "algo" and "delimiter" rows get persisted.
9022+
sqlite3 *db = NULL;
9023+
int rc = sqlite3_open(dbpath, &db);
9024+
if (rc != SQLITE_OK) return false;
9025+
sqlite3_exec(db, "PRAGMA journal_mode=WAL;", NULL, NULL, NULL);
9026+
sqlite3_cloudsync_init(db, NULL, NULL);
9027+
9028+
rc = sqlite3_exec(db, "CREATE TABLE docs (id TEXT PRIMARY KEY NOT NULL, body TEXT);", NULL, NULL, NULL);
9029+
if (rc != SQLITE_OK) goto cleanup;
9030+
9031+
rc = sqlite3_exec(db, "SELECT cloudsync_init('docs');", NULL, NULL, NULL);
9032+
if (rc != SQLITE_OK) goto cleanup;
9033+
9034+
rc = sqlite3_exec(db, "SELECT cloudsync_set_column('docs', 'body', 'algo', 'block');", NULL, NULL, NULL);
9035+
if (rc != SQLITE_OK) goto cleanup;
9036+
9037+
rc = sqlite3_exec(db, "SELECT cloudsync_set_column('docs', 'body', 'delimiter', '\n');", NULL, NULL, NULL);
9038+
if (rc != SQLITE_OK) goto cleanup;
9039+
9040+
// Insert something so the blocks table is populated.
9041+
rc = sqlite3_exec(db, "INSERT INTO docs (id, body) VALUES ('doc1', 'AAA\nBBB\nCCC');", NULL, NULL, NULL);
9042+
if (rc != SQLITE_OK) goto cleanup;
9043+
9044+
int64_t settings_before = do_select_int(db, "SELECT count(*) FROM cloudsync_table_settings WHERE tbl_name='docs' AND col_name='body';");
9045+
9046+
sqlite3_exec(db, "SELECT cloudsync_terminate();", NULL, NULL, NULL);
9047+
sqlite3_close(db);
9048+
db = NULL;
9049+
9050+
// Phase 2: reopen — sqlite3_cloudsync_init triggers dbutils_settings_load,
9051+
// which iterates cloudsync_table_settings and replays the block-column setup.
9052+
// Without the fix this hangs (infinite REPLACE-during-iteration loop).
9053+
rc = sqlite3_open(dbpath, &db);
9054+
if (rc != SQLITE_OK) goto cleanup;
9055+
rc = sqlite3_cloudsync_init(db, NULL, NULL);
9056+
if (rc != SQLITE_OK) goto cleanup;
9057+
9058+
// Settings table should be unchanged after reload (no duplicate rewrites).
9059+
int64_t settings_after = do_select_int(db, "SELECT count(*) FROM cloudsync_table_settings WHERE tbl_name='docs' AND col_name='body';");
9060+
if (settings_after != settings_before) {
9061+
printf("block_column_reload: settings count changed across reload (before=%" PRId64 ", after=%" PRId64 ")\n", settings_before, settings_after);
9062+
goto cleanup;
9063+
}
9064+
9065+
// Block column wiring must still be active after reload: an UPDATE should
9066+
// continue to write per-block metadata, not a single whole-column entry.
9067+
rc = sqlite3_exec(db, "UPDATE docs SET body='AAA\nXXX\nCCC' WHERE id='doc1';", NULL, NULL, NULL);
9068+
if (rc != SQLITE_OK) goto cleanup;
9069+
9070+
int64_t whole_meta = do_select_int(db, "SELECT count(*) FROM docs_cloudsync WHERE col_name='body';");
9071+
if (whole_meta != 0) {
9072+
printf("block_column_reload: expected 0 whole-column metadata entries after reload, got %" PRId64 "\n", whole_meta);
9073+
goto cleanup;
9074+
}
9075+
9076+
result = true;
9077+
9078+
cleanup:
9079+
if (db) {
9080+
sqlite3_exec(db, "SELECT cloudsync_terminate();", NULL, NULL, NULL);
9081+
sqlite3_close(db);
9082+
}
9083+
if (cleanup_databases) {
9084+
file_delete_internal(dbpath);
9085+
char walpath[280];
9086+
snprintf(walpath, sizeof(walpath), "%s-wal", dbpath);
9087+
file_delete_internal(walpath);
9088+
snprintf(walpath, sizeof(walpath), "%s-shm", dbpath);
9089+
file_delete_internal(walpath);
9090+
}
9091+
return result;
9092+
}
9093+
90049094
static char *do_select_text(sqlite3 *db, const char *sql) {
90059095
sqlite3_stmt *stmt = NULL;
90069096
char *val = NULL;
@@ -12056,6 +12146,7 @@ int main (int argc, const char * argv[]) {
1205612146
result += test_report("Large Composite PK Test:", do_test_large_composite_pk(2, print_result, cleanup_databases));
1205712147
result += test_report("Schema Hash Mismatch:", do_test_schema_hash_mismatch(2, print_result, cleanup_databases));
1205812148
result += test_report("Stale Table Settings:", do_test_stale_table_settings(cleanup_databases));
12149+
result += test_report("Block Column Reload:", do_test_block_column_reload(cleanup_databases));
1205912150
result += test_report("CB Error Cleanup:", do_test_context_cb_error_cleanup());
1206012151

1206112152
finalize:

0 commit comments

Comments
 (0)