-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sqlite3.exec throws "RuntimeError: memory access out of bounds" #156
Comments
Answers to your questions:
Some questions for you:
|
That's roughly 30K queries, yes? Is it possible to log all the SQL you submit, including any setup, so we can just replay it and reproduce the error that way? Or even better, can you write a succinct generator that will initialize the schema and database and produce an arbitrary number of simulated queries? |
I will put together a detailed answer to your questions tomorrow. A dedicated Chromebook is always on and only running my extension. I rebuilt wa-sqlite with |
Asyncify
Default
Yes, using 11 modules.
exec() and virtual tables call result() inside xColumn() function
It can change, but it is the same set of queries 95% of the time.
Only reads -- only SELECT statements.
Dedicated Chromebook.
No For reference, the relevant extension code is here: I'll try and see if I can reproduce this issue faster. |
I created a repo reproducing this issue: https://github.com/getvictor/fleetd-chrome And video instructions: https://www.loom.com/share/603cb991739f4e54b1603e7fed1ff273?sid=e31ab9ee-9660-43bb-b477-9146b8fedff3 My build with |
I can see an error with the repo and the instructions. With a debug wa-sqlite build, I get:
ASYNCIFY_STACK_SIZE was one of my top suspects when I learned this is an Asyncify build. But this is a very simple query so I don't see why that should take a lot of stack space, and when I increased ASYNCIFY_STACK_SIZE all the way up to 8 MB (from 16 KB) the error persists. So I think this is not the problem. Or perhaps it could be part of the problem but not the only problem. My other top suspect was memory leak. This also seems less likely now that it doesn't take days to reproduce. I have looked over the My new top suspect is one I didn't consider earlier: actual memory corruption in the virtual table structs. It's possible there is an error in translating JavaScript objects to C structs. I'll follow up on that. @getvictor In the reproduction repo, is there anything in those virtual tables when the error happens or are they empty? |
The virtual tables should not be persisting any data. Their return values are either hardcoded, or they call chrome APIs to get their values. I think their values have to be re-translated from JS to C and back to JS on every SELECT. |
This comment was marked as outdated.
This comment was marked as outdated.
@getvictor I suspect this occurs because your virtual table modules throw an error here. It looks like when this happens the WebAssembly stack is not properly unwound and subsequent calls into WebAssembly will start with that part of the stack unavailable. Eventually all these now inaccessible stack regions take up all the allotted stack space. Can you please try returning a valid SQLite result code from all module methods, and see if that fixes the problem? Investigative details follow: I patched my SQLite source to log the stack address when SQLITE_API int sqlite3_prepare_v2(
sqlite3 *db, /* Database handle. */
const char *zSql, /* UTF-8 encoded SQL statement. */
int nBytes, /* Length of zSql in bytes. */
sqlite3_stmt **ppStmt, /* OUT: A pointer to the prepared statement */
const char **pzTail /* OUT: End of parsed string */
){
int rc;
printf("sqlite3_prepare_v2 %p %s\n", &rc, zSql);
... A couple iterations of output looks like this:
We expect the address when entering from JavaScript always to be the same, but instead we see it is lower after accessing network_interfaces, screenlock, and system_state virtual tables. Those specific virtual tables are suspiciously labeled as "bad_tables". Error logging from these tables is suppressed, but it appears likely these unlogged exceptions are skipping stack unwinding. That might be an Emscripten bug, or it might just be that you aren't supposed to do that. |
Removing |
Filed an Emscripten issue in case this is a bug. Update: Not an Emscripten bug - don't throw on calls from SQlite WebAssembly to JavaScript, that produces undefined behavior. |
#16394 As suggested: rhashimoto/wa-sqlite#156 (comment) # Checklist for submitter If some of the following don't apply, delete the relevant line. <!-- Note that API documentation changes are now addressed by the product design team. --> - [x] Changes file added for user-visible changes in `changes/` or `orbit/changes/`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Manual QA for all new/changed functionality
We have a Chrome extension using wa-sqlite (latest versions), and some Chromebooks are seeing:
The error typically takes ~2 days to show up, where the database is used for ~10 queries per minute. Once this error happens, it continues happening for each subsequent
exec
call.I'm not familiar with WASM. Some thoughts/questions:
TOTAL_STACK=2mb
The text was updated successfully, but these errors were encountered: