Skip to content

Commit 651fad3

Browse files
committed
Simpler sqlite3_finalize patch: try_lock only, keep stmt_run_to_completion
Previous variant (skip stmt_run_to_completion + early-return on try_lock failure) double-freed the stmt and segfaulted. Use a minimal patch: replace .lock().unwrap() with if-let Ok(try_lock). On contention the stmt_list cleanup is skipped (potential small leak per affected stmt), but destructors and free still run.
1 parent 10501ae commit 651fad3

1 file changed

Lines changed: 44 additions & 30 deletions

File tree

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

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,46 +62,60 @@ 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.
65+
# Turso's sqlite3_finalize blocks indefinitely on sqlite3Inner's
66+
# mutex during PHP shutdown. Replace .lock().unwrap() with try_lock:
67+
# on contention, skip the stmt_list cleanup (small leak) rather
68+
# than block forever. The rest of finalize (destructors, free) still
69+
# runs.
7370
python3 - <<'PY_FIN'
7471
path = 'sqlite3/src/lib.rs'
7572
src = open(path).read()
76-
7773
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"
74+
" if !stmt_ref.db.is_null() {\n"
75+
" let db = &mut *stmt_ref.db;\n"
76+
" let mut db_inner = db.inner.lock().unwrap();\n"
77+
"\n"
78+
" if db_inner.stmt_list == stmt {\n"
79+
" db_inner.stmt_list = stmt_ref.next;\n"
80+
" } else {\n"
81+
" let mut current = db_inner.stmt_list;\n"
82+
" while !current.is_null() {\n"
83+
" let current_ref = &mut *current;\n"
84+
" if current_ref.next == stmt {\n"
85+
" current_ref.next = stmt_ref.next;\n"
86+
" break;\n"
87+
" }\n"
88+
" current = current_ref.next;\n"
89+
" }\n"
90+
" }\n"
8491
" }\n"
8592
)
8693
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"
94+
" if !stmt_ref.db.is_null() {\n"
95+
" let db = &mut *stmt_ref.db;\n"
96+
" // try_lock: on contention, skip stmt_list cleanup to\n"
97+
" // avoid deadlocking in PHP's GC-driven shutdown path.\n"
98+
" if let Ok(mut db_inner) = db.inner.try_lock() {\n"
99+
" if db_inner.stmt_list == stmt {\n"
100+
" db_inner.stmt_list = stmt_ref.next;\n"
101+
" } else {\n"
102+
" let mut current = db_inner.stmt_list;\n"
103+
" while !current.is_null() {\n"
104+
" let current_ref = &mut *current;\n"
105+
" if current_ref.next == stmt {\n"
106+
" current_ref.next = stmt_ref.next;\n"
107+
" break;\n"
108+
" }\n"
109+
" current = current_ref.next;\n"
110+
" }\n"
111+
" }\n"
112+
" }\n"
113+
" }\n"
99114
)
100-
assert old in src, 'finalize lock not found'
115+
assert old in src, 'finalize block not found'
101116
src = src.replace(old, new, 1)
102-
103117
open(path, 'w').write(src)
104-
print('patched sqlite3_finalize (skip run_to_completion, try_lock)')
118+
print('patched sqlite3_finalize (try_lock)')
105119
PY_FIN
106120
107121
python3 - <<'PY'

0 commit comments

Comments
 (0)