Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) by CraigBuckmaster · Pull Request #1534 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) by CraigBuckmaster · Pull Request #1534 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) by CraigBuckmaster · Pull Request #1534 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) by CraigBuckmaster · Pull Request #1534 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) by CraigBuckmaster · Pull Request #1534 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) by CraigBuckmaster · Pull Request #1534 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) by CraigBuckmaster · Pull Request #1534 · CraigBuckmaster/ScriptureDeepDive · GitHub
Skip to content

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash) - #1534

Merged
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash
Apr 20, 2026
Merged

fix(ContentUpdater): remove full-DB PRAGMA integrity_check (iOS 26 FTS5 teardown crash)#1534
CraigBuckmaster merged 1 commit into
masterfrom
fix/remove-integrity-check-fts5-crash

Conversation

@CraigBuckmaster

Copy link
Copy Markdown
Owner

Why

TestFlight 1.0.7(20) segfaulted at launch. The crash is in SQLite's FTS5 teardown path during closeAsync, triggered by the PRAGMA integrity_check call we added in #1529.

Crash signature

EXC_BAD_ACCESS (SIGSEGV), 3.5s after launch, Thread 7:

exsqlite3_finalize ← garbage pointer deref
sqlite3Fts5IndexClose
fts5DisconnectMethod
fts5FreeVtab
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase ← expo-sqlite iOS closeAsync

What's happening

  1. Download completes → native File.downloadFileAsync writes temp file (fine, no memory spike — that fix from fix + diagnostic: native DB download, correct startup ordering, JS error handler #1529 is still working)
  2. We open the temp DB with openDatabaseAsync(tempDbName)
  3. We run SELECT content_hash FROM db_meta — passes
  4. We run PRAGMA integrity_check — passes
  5. verifyDb.closeAsync() → SQLite tears down FTS5 vtabs → finalize on stale pointer → segfault

integrity_check forces SQLite to load and validate every FTS5 index. scripture.db has FTS5 tables for verse search and people search. The combination of "FTS5 indexes loaded by integrity_check" + "expo-sqlite's closeAsync path on iOS 26" produces a use-after-free during teardown.

The fix

Remove the PRAGMA integrity_check block from downloadFullDb. Keep the content_hash check — that hash is baked into db_meta at build time over the full file contents, so a corrupt/truncated download fails at open or misses the row. Less rigorous than integrity_check, but avoids the teardown crash.

Documented as a TODO to revisit once expo-sqlite's FTS5 teardown is fixed upstream, or to migrate to streaming SHA-256 during the chunked write.

Intentionally NOT fixed here

The applyDelta path has the same PRAGMA integrity_checkcloseAsync pattern at ContentUpdater.ts:234. Left as-is because:

  • First-launch users (the ones currently crashing) never hit the delta path
  • Delta integrity_check runs after transactional SQL mutation — more valuable there as a corruption check
  • If it ever crashes for a delta user, same fix applies; deferring avoids scope creep

A separate follow-up PR can address the delta-path integrity_check if it becomes a problem.

Tests

  • Removed returns failed when integrity_check fails after download (the tested code path no longer exists)
  • Removed dead integrity_check mocks from 5 full-DB tests where they'd never be consumed now
  • Delta-path tests untouched
  • 35 ContentUpdater tests pass
  • Typecheck clean, lint 0 errors

Changes against PR #1529 that stay intact

  • Native download for large payloads (the real memory fix) ✓
  • content_hash verification ✓
  • Hydration ordering fix ✓
  • onComplete error surfacing ✓
  • Global JS error handler diagnostic ✓

Expected build 21 behavior

Three outcomes possible:

Supersedes

Budget

~$4-6 EAS credits remain. This will be the final build of the month likely. If it works, great. If not, we have the diagnostic and crash logs to plan next month's work carefully.

…S5 teardown crash)
## The crash
TestFlight 1.0.7(20) segfaulted 3.5s after launch — EXC_BAD_ACCESS
on Thread 7, triggered by:
exsqlite3_finalize
sqlite3Fts5IndexClose
fts5DisconnectMethod
sqlite3VtabUnlock
disconnectAllVtab
sqlite3Close
SQLiteModule.closeDatabase (expo-sqlite iOS, SQLiteModule.swift:489)
The call chain: SDK 54 expo-sqlite's closeAsync → sqlite3Close →
disconnectAllVtab → iterates every FTS5 vtab → fts5DisconnectMethod
→ fts5FreeVtab → sqlite3Fts5IndexClose → exsqlite3_finalize on a
stale/freed statement pointer → crash.
This is a teardown-path bug in FTS5 virtual-table cleanup that
manifests specifically when:
1. The DB has FTS5 tables (scripture.db has them for verse/people search)
2. PRAGMA integrity_check was run on the connection (which loads
and validates every FTS5 index, leaving them in a state that
doesn't clean up correctly)
3. closeAsync is called on that connection
Our full-DB download verification path added by #1529 (cherry-pick
from #1531 commit 1) did exactly that: open temp DB, run content_hash
check, run integrity_check, close. The integrity_check was the
trigger.
## The fix
Remove the PRAGMA integrity_check from the full-DB post-download
verification path. Keep the content_hash check — it's sufficient
validation: the hash was computed at build time over the full file
contents, baked into db_meta at build time, and uploaded with the
DB to R2. A corrupt/truncated download is overwhelmingly likely to
fail at SQLite open (exception thrown before our read) or to miss
the db_meta row entirely.
This is less rigorous than integrity_check but avoids the teardown
crash. TODO: re-add proper integrity verification once expo-sqlite's
FTS5 teardown bug is resolved upstream, or migrate to streaming
SHA-256 during the chunked write.
## Scope
INTENTIONALLY LIMITED. The delta-apply path in applyDelta() has
the same latent PRAGMA integrity_check → close pattern (~line 234).
Left in place because:
- Delta updates don't fire for first-launch users (the crash we're
chasing is build-20 first-launch)
- Delta integrity_check runs AFTER transactional SQL mutation, so
it's more valuable as a corruption check than the post-download
one was
- Touching it expands scope beyond the confirmed crash site
- If it ever DOES crash for a delta user, same fix applies; we'll
know when we see the crash log
## Tests
- Removed 'returns failed when integrity_check fails after download'
(asserted behavior that no longer exists)
- Removed 5 dead integrity_check mocks from full-DB tests where they
were set up but would never be consumed now
- Delta-path tests untouched (still exercise integrity_check)
- 35 ContentUpdater tests pass (was 36, minus 1 removed)
- tsc clean, lint 0 errors
## Build budget
Build 21 will be the 2nd build on master since #1529 shipped. With
no other changes, failure mode cascade from the cascade in #1529:
- If Alert shows on relaunch → a JS error got past the fixes.
Handler captured it. We see it next launch.
- If app reaches home screen → all three crash causes fixed.
We revert the diagnostic and ship.
- If app still crashes silently → exceptionally rare; a different
native-layer issue we haven't hit yet.
## References
- Crash incident: 8C94684A-D8BE-45F5-B9A7-412450196AC4
- iOS Analytics log: CompanionStudy-2026-04-19-..-1.0.7(20)
- Supersedes PR #1533 (which kept the integrity_check call in place)
@github-actions

Copy link
Copy Markdown

Test Results

✅ All tests passed

PassedFailedTotal
Tests✅ 3431❌ 03431
Suites✅ 465❌ 0465

Coverage

StatementsBranchesFunctionsLines

⏱️ Duration: 78.8s

@CraigBuckmaster
CraigBuckmaster merged commit 39a9590 into masterApr 20, 2026
6 checks passed
@CraigBuckmaster
CraigBuckmaster deleted the fix/remove-integrity-check-fts5-crash branch April 20, 2026 00:17
CraigBuckmaster pushed a commit that referenced this pull request Apr 20, 2026
…tion
## Safety valve
Previously, closeDatabaseConnection() swallowed errors from closeAsync
and always set db = null, reporting success. If the native SQLite close
actually failed (e.g., a statement still in flight), the subsequent
file-swap would then move a temp file over a DB that SQLite still held
open — leading to lock errors or, on iOS 26 with FTS5 tables, the
teardown crash that PR #1534 just fixed for a different code path.
Now closeDatabaseConnection() returns Promise<boolean>:
- true : close succeeded OR db was already null (safe to swap)
- false : closeAsync threw (NOT safe to swap)
ContentUpdater.applyDelta and .downloadFullDb both check the return
value. On false, they:
1. Log a warning
2. Cleanup any temp files (full-DB path)
3. Return { status: 'up_to_date' }
The next OTA check will try again. Failing an update is strictly
better than corrupting the DB file.
## Option B TODO
Added a 30-line documentation block above ContentUpdaterService
explaining the long-term architectural direction: defer-to-cold-start
instead of close-and-reopen. Rationale:
- The ONLY fully-safe time to swap the DB file is when no SQLite
connection exists at all.
- That condition naturally occurs during app cold-start, before
initDatabase() runs.
- Current close-and-reopen creates that condition artificially mid-
session, which is fragile (stale getDb() refs, in-flight query
errors).
- Defer-to-cold-start trades 'updates land immediately' for
'updates land on next launch', which is acceptable for scholarly
content updates.
This PR ships phase 1 (close-and-reopen with safety valve). Phase 2
(full cold-start-swap architecture) is a separate project tracked as
a TODO comment. PR #1533's original 'defer OTA when live DB open'
idea informed this direction but cannot be implemented as-is (the
DB is always open after startup, so defer-on-live-db would mean
updates never fire).
## Tests
- Added 2 focused tests covering the safety valve:
- aborts delta apply when live DB close fails
- aborts full DB swap when live DB close fails
Both assert status=up_to_date AND that the swap/backup was NOT
attempted (mockFileOps.move / copy).
- Updated mock default return values for closeDatabaseConnection
from undefined to true (matches new signature).
- 41 ContentUpdater tests pass (was 39 + 2 new).
- Broader sweep: 30 suites / 454 tests pass.
- Typecheck clean, lint 0 errors.
## Not in scope
Cold-start-swap implementation is out of scope for this PR — that's
a separate project. This PR keeps #1532's original close-and-reopen
strategy and hardens it; it does not replace it.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@CraigBuckmaster