Skip to content

Commit 10501ae

Browse files
committed
Patch sqlite3_finalize to avoid deadlock, re-enable temp_master patch
Patch Turso's sqlite3_finalize: - Skip stmt_run_to_completion — pdo_sqlite is discarding the stmt anyway, and this is where an infinite step loop can happen. - Use try_lock on db.inner instead of .lock().unwrap(); on contention, bail out (small stmt_list leak) rather than block forever. With the deadlock avoided, re-enable temporary_table_exists → false to reclaim the ~250 sqlite_temp_master errors.
1 parent 8c01165 commit 10501ae

1 file changed

Lines changed: 73 additions & 19 deletions

File tree

.github/workflows/phpunit-tests-turso.yml

Lines changed: 73 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,48 @@ jobs:
6262
run: |
6363
sed -i 's|todo!("{} is not implemented", stringify!($fn));|return unsafe { std::mem::zeroed() };|' sqlite3/src/lib.rs
6464
65+
# Turso's sqlite3_finalize deadlocks when PHP's garbage collector
66+
# runs it during shutdown: sqlite3Inner's mutex is held by a Turso
67+
# async thread that stays alive past the stmt's lifetime.
68+
# 1) Skip stmt_run_to_completion — pdo_sqlite is discarding the
69+
# statement, so finishing execution gains us nothing.
70+
# 2) Replace .lock().unwrap() on the db inner with try_lock: if
71+
# another holder exists, skip the stmt_list cleanup (small leak)
72+
# rather than block forever.
73+
python3 - <<'PY_FIN'
74+
path = 'sqlite3/src/lib.rs'
75+
src = open(path).read()
76+
77+
old = (
78+
" // first, finalize any execution if it was unfinished\n"
79+
" // (for example, many drivers can consume just one row and finalize statement after that, while there still can be work to do)\n"
80+
" // (this is necessary because queries like INSERT INTO t VALUES (1), (2), (3) RETURNING id return values within a transaction)\n"
81+
" let result = stmt_run_to_completion(stmt);\n"
82+
" if result != SQLITE_OK {\n"
83+
" return result;\n"
84+
" }\n"
85+
)
86+
new = (
87+
" // (stmt_run_to_completion elided to avoid a deadlock in\n"
88+
" // PHP's garbage-collected sqlite3_finalize path.)\n"
89+
)
90+
assert old in src, 'stmt_run_to_completion call not found'
91+
src = src.replace(old, new, 1)
92+
93+
old = " let mut db_inner = db.inner.lock().unwrap();\n"
94+
new = (
95+
" let mut db_inner = match db.inner.try_lock() {\n"
96+
" Ok(g) => g,\n"
97+
" Err(_) => { let _ = Box::from_raw(stmt); return SQLITE_OK; }\n"
98+
" };\n"
99+
)
100+
assert old in src, 'finalize lock not found'
101+
src = src.replace(old, new, 1)
102+
103+
open(path, 'w').write(src)
104+
print('patched sqlite3_finalize (skip run_to_completion, try_lock)')
105+
PY_FIN
106+
65107
python3 - <<'PY'
66108
import re
67109
@@ -428,25 +470,37 @@ jobs:
428470
working-directory: packages/mysql-on-sqlite
429471
run: |
430472
python3 - <<'PY'
431-
# Driver patches ATTEMPTED and abandoned:
432-
#
433-
# - temporary_table_exists → false: would save ~250 errors from
434-
# 'no such table: sqlite_temp_master', but changing the predicate's
435-
# return value makes the driver take code paths that deadlock inside
436-
# Turso's sqlite3_finalize during subsequent tests (gdb traces show
437-
# a mutex on sqlite3Inner held across the step->finalize cycle).
438-
# Requires a Turso fix.
439-
#
440-
# - sync_column_key_info row-value UPDATE rewrite: valid SQL, but
441-
# triggers a pathological case in Turso's query planner that hangs
442-
# PHPUnit somewhere after test 504/667.
443-
#
444-
# Only the wp_die polyfill is safe; it doesn't change driver/query
445-
# behaviour.
446-
447-
# wp_die polyfill: real WordPress provides wp_die(); the unit-test
448-
# bootstrap doesn't, so 21 tests hit an 'undefined function' error
449-
# when a driver error path tries to call it.
473+
# 1. Turso doesn't expose `sqlite_temp_master`. The driver queries it
474+
# to decide whether to use temp-schema tables, but Turso doesn't
475+
# support TEMP tables at all, so the predicate is always false.
476+
path = 'src/sqlite/class-wp-sqlite-information-schema-builder.php'
477+
src = open(path).read()
478+
old = (
479+
"\tpublic function temporary_table_exists( string $table_name ): bool {\n"
480+
"\t\t/*\n"
481+
"\t\t * We could search in the \"{$this->temporary_table_prefix}tables\" table,\n"
482+
"\t\t * but it may not exist yet, so using \"sqlite_temp_master\" is simpler.\n"
483+
"\t\t */\n"
484+
"\t\t$stmt = $this->connection->query(\n"
485+
"\t\t\t\"SELECT 1 FROM sqlite_temp_master WHERE type = 'table' AND name = ?\",\n"
486+
"\t\t\tarray( $table_name )\n"
487+
"\t\t);\n"
488+
"\t\treturn $stmt->fetchColumn() === '1';\n"
489+
"\t}"
490+
)
491+
new = (
492+
"\tpublic function temporary_table_exists( string $table_name ): bool {\n"
493+
"\t\treturn false; // Turso has no TEMP tables and no sqlite_temp_master.\n"
494+
"\t}"
495+
)
496+
assert old in src, 'temporary_table_exists body not found'
497+
src = src.replace(old, new, 1)
498+
open(path, 'w').write(src)
499+
print('patched information-schema-builder.php')
500+
501+
# 2. wp_die polyfill: real WordPress provides wp_die(); the unit-test
502+
# bootstrap doesn't, so 21 tests hit an 'undefined function' error
503+
# when a driver error path tries to call it.
450504
path = 'tests/bootstrap.php'
451505
src = open(path).read()
452506
marker = "if ( ! function_exists( 'do_action' ) ) {"

0 commit comments

Comments
 (0)