Fix changeset_apply crash: use proper WASM function pointer for conflict handler#1021
Open
acusti wants to merge 1 commit intolivestorejs:devfrom
Open
Fix changeset_apply crash: use proper WASM function pointer for conflict handler#1021acusti wants to merge 1 commit intolivestorejs:devfrom
acusti wants to merge 1 commit intolivestorejs:devfrom
Conversation
The conflict handler passed to sqlite3changeset_apply was a plain JS function, but cwrap declares all parameters as 'number'. The function was silently coerced to 0 (null pointer), causing a "function signature mismatch" RuntimeError whenever SQLite invoked the conflict callback during rebase rollback. Fix: use Module.addFunction() to register the conflict handler as a proper WASM function pointer. The pointer is created lazily on first changeset_apply call and cached for reuse, so environments that disallow dynamic WebAssembly compilation (e.g. Cloudflare Workers) don't crash during Factory() init if changeset_apply is never called. Also add -sALLOW_TABLE_GROWTH to the Emscripten build flags so the WASM function table can accommodate the new addFunction call (the current binaries ship with table max == table min, leaving zero room). Fixes livestorejs#998 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #998
sqlite3.changeset_applypasses a conflict handler callback to the C functionsqlite3changeset_apply. The current code passes a plain JS function, butcwrapdeclares all parameters as'number', so the function is silently coerced to0(null pointer). This causes aRuntimeError: function signature mismatchwhenever SQLite invokes the conflict callback during rebase rollback.Changes
packages/@livestore/wa-sqlite/src/sqlite-api.jsModule.addFunction()to register the conflict handler as a proper WASM function pointerchangeset_applycall and cached for reuse, so environments that disallow dynamic WebAssembly compilation (e.g. Cloudflare Workers) don't crash duringFactory()init ifchangeset_applyis never calledpackages/@livestore/wa-sqlite/Makefile-sALLOW_TABLE_GROWTH=1toEMFLAGS_COMMONso the WASM function table can accommodate theaddFunctioncall (the current binaries ship withtable max == table min, leaving zero room for runtime function registration)Notes
ALLOW_TABLE_GROWTH,addFunctionfails because the function table can't grow.addFunctioncallsconvertJsFunctionToWasm, which dynamically compiles a small WASM module. This is fine in browsers and Node.js, but Cloudflare Workers disallow dynamicWebAssembly.Module()construction. By deferring to first use, environments that import but never callchangeset_apply(e.g. Durable Objects using native SQLite) aren't affected.Test plan
RuntimeError: function signature mismatchcrash in browserCompileError: Wasm code generation disallowed by embedder)Responsible Disclosure
The code changes and description in this PR were created by Claude Code Opus 4.6. I reviewed the changes but I don’t have any experience with WASM, so I’m way out of my depth here. The first version of the fix for this created the onConflictPtr (
const onConflictPtr = Module.addFunction(...)) directly in the IIFE (above the returned function), like:but that triggered errors in my DO client:
Error: (FiberFailure) CompileError: WebAssembly.Module(): Wasm code generation disallowed by embedderHence the lazy initialization approach taken here, which keeps the DO from crashing as long as it doesn’t hit the
changeset_applycodepath.