-
Notifications
You must be signed in to change notification settings - Fork 269
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
WASM file crashes Google Chrome #1
Comments
@gziolo captured this issue on the video: Screen.Recording.2022-09-21.at.11.42.01.mov |
Here's Birgit's videos capturing the crash: https://recordit.co/0gMsH3GDn1 |
Interestingly, I can't get it to crash in Firefox or Safari. Perhaps we're hitting a Chrome bug? |
Here's an idea: compile Chromium with
Conveniently, on Ubuntu the debug symbols are shipped as a part of the Even better – instead of attaching the debugger, inspect the minidump file generated by chromium on crash: |
I debugged Chromium today and added my findings to this issue's description: #1 (comment) CC @jsnajdr and @dmsnell – you might enjoy exploring this challenge with me. Also cc @swissspidy in case you know anyone in Chromium team who might be willing to take a look at this. |
Chrome debugging aside, the php-wasm playground seems to just work and never break – I wonder why is that. One difference I see is they don't use web workers and just load everything in the main thread. |
I think I'm getting somewhere – when PHP is running in the main thread and not in a webworker, it never seems to crash. I wonder if this is related to wasm at all, or is it just some inter-process issue with web workers. |
@adamziel I have no idea, but if it's related to a |
@dmsnell I started with a single-threaded setup, but it was super slow: every request took forever to load AND your interactions with the page were blocked while it was being loaded. My hypothesis is that the speed penalty was due to forcing Chrome to switch contexts between rendering and handling WASM. I'm exploring a minimal reproducible crash scenario. Turns out, a web worker like this is all it takes: console.log( '[WebWorker] Spawned' );
importScripts( '/webworker-php.js' ); // Generated by emscripten
new PHP( {} ) // PHP is the generated module
.then( () => {
console.log( '[WebWorker] PHP initialized' );
} ); Note I'm not transferring anything between threads explicitly, although something could be happening implicitly. It could be something super specific deep in the emscripten's loading setup. I'll give it an hour more or so, but if I can't pinpoint the issue then I'll explore emulating a webworker with an iframe. |
The crash happens when instantiating Booting WordPress WASM goes through the following emscripten-generated code path: fetch( wasmBinaryFile, { credentials: 'same-origin' } ).then( function(
response,
) {
const result = WebAssembly.instantiateStreaming( response, info );
return result.then( receiveInstantiatedSource, function( reason ) {
err( 'wasm streaming compile failed: ' + reason );
err( 'falling back to ArrayBuffer instantiation' );
return instantiateArrayBuffer( receiveInstantiatedSource );
} );
} ); The crash still occurs after commenting // Crashes
WebAssembly.instantiateStreaming(
await fetch( wasmBinaryFile ),
info,
);
// Regular instantiate() crashes as well
fetch( wasmBinaryFile ).then( async ( response ) => {
WebAssembly.instantiate(
await response.arrayBuffer(),
info,
);
} ); Here's an exploratory branch where the crash is being boiled down to its essence. Feel free to clone and help with this one. So how to fix it? Here's a few ideas:
Accordingly to this StackOverflow answer, most major browsers seem to run iframes in a separate thread as long as it comes from a different domain. That's not a part of any spec, though. Just an implementation detail that may change at any time. If the best solution still incurs a speed penalty it could become a chrome-only fallback. |
It must be specific to the php wasm build. I just built a minimal .wasm from the following #include <emscripten.h>
#include <stdlib.h>
int main() { return 0; }
int EMSCRIPTEN_KEEPALIVE test()
{
return 10;
} A few ideas where to go from here:
It would also be great to prepare an isolated branch with a minimal reproduction example and loop in the chrome team. Edit: I compiled just Here's what else I just tried without success:
|
A combination of the following
Importantly, it's only 711KB instead of 10MB – most of the symbols must have been optimized away. That's good progress, though, the build process with and without a crash is clear – it should be possible to zero-in on the cause. |
I don't think optimization is going to impact the stability of the build. in fact, if you try there's another what about trying to run this in |
The crash is caused by the wasm-ized PHP function called I played with the PHP C code for a day, but couldn't identify the root cause of the crash. I did however reduce the crashing binary size from 10 MB to 600 KB. From there, I was able to convert it to a WAT text format using
@dmsnell unfortunately I couldn't cause the crash with |
Oh I forgot to mention – that "minimal" 122KB file is literally just the |
this is good work; I hope it leads to fixes if there's a problem with the WASM runtime |
Here's the .wat file I promised earlier: Everything you have to do to cause the crash is compile it to (() => {
// src/web/web-worker.js
console.log("[WebWorker] Spawned");
var wasmTable = new WebAssembly.Table({
initial: 1090,
maximum: 1090,
element: "anyfunc"
});
var WASM_PAGE_SIZE = 65536;
var INITIAL_INITIAL_MEMORY = 1073741824;
var wasmMemory = new WebAssembly.Memory({
initial: INITIAL_INITIAL_MEMORY / WASM_PAGE_SIZE
});
var info = {
env: {
_zend_empty_array2: 1,
tempDoublePtr: 2303696,
"__memory_base": 1024,
__table_base: 0,
memory: wasmMemory,
table: wasmTable
},
global: { NaN: NaN, Infinity: Infinity },
asm2wasm: {
"f64-rem"() {
}
}
};
fetch("updated.wasm").then(async (response) => {
WebAssembly.instantiate(
await response.arrayBuffer(),
info
).then(() => {
console.log("Instantiated!");
});
console.log("Called instantiate");
});
console.log("Called fetch", { info });
})(); In the meantime, I keep reducing it further to isolate the specific part causing the problem. |
I filed a Chromium issue (it seems to be private). Minidump/crash ID: dfe74270-7a4e-4145-9859-e77979a8145d |
I got it down to 2.6 M, see minimal_repro.zip. To try, host it locally e.g. via Reducing the How complex? Well, this is how that function starts: |
Here's the current situation:
I don't know how else to zero-in on the root cause of the crash, so I'll try to work around it without understanding it well. I can only thing of two ways:
|
Running WebAseembly inside of an iframe sourced from another domain is decently fast and does not crash Chrome! The downside is a multi-domain setup which takes much more work than just adding a I will update trunk to use iframes instead of webworkers and leave solving the crash to the Chrome team. See my previous comment for the details of the related bugs.chromium.org issue. |
#28 works around the issue by handling the WASM in an iframe coming from a different domain name. |
For posterity, this seems to be an Out of Memory problem in v8 – it's weird it never occurs in node.js. The Chromium team shared the following stack trace generated on crash:
|
Load WASM in an iframe instead of a Webworker to work around a Google Chrome out of memory error. This commit enables defering the WASM workload to a configurable backend. It ships with three backends: `iframeWorkerBackend`, `webWorkerBackend`, `sharedWorkerBackend`. The `iframeWorkerBackend` is the default. See #1 for more details about the Google Chrome crash. **Important!** The iframe must be loaded from another domain to spin a new browser thread. If it's loaded from the same domain, WordPress "server" will run in the same thread that paints the user interface and dramatically slow down all user interactions.
I'm attaching a reproduction for posterity. It consists of two HTML files: |
I also played with compiling PHP into a WASM file with less nested blocks. The The grammar file lives in yy11:
YYDEBUG(11, *YYCURSOR);
YYCURSOR = YYMARKER;
goto yy7;
yy12:
YYDEBUG(12, *YYCURSOR);
yych = *++YYCURSOR;
if (yych == 'P') goto yy13;
if (yych != 'p') goto yy11;
yy13:
YYDEBUG(13, *YYCURSOR);
yych = *++YYCURSOR;
if (yych <= '\f') {
if (yych <= 0x08) goto yy11;
if (yych >= '\v') goto yy11;
} else {
if (yych <= '\r') goto yy16;
if (yych != ' ') goto yy11; I thought there should be a way to generate a loop-based code without any gotos. The version I generated a loop-based parser, compiled PHP to wasm, and inspected the output. The good news is it didn't have the same level of nesting. The bad news is it was much worse. The loop-based parser made for ~800 levels of nested blocks instead of ~100: I'm now starting to think there's no reducing the nesting in that To summarize:
I don't think there's anything else we can do here other than wait for a bugfix from the Chrome team. Until that happens, this project will use iframes instead of web workers to offload WASM processing. |
Oh and one last note – it also crashes in a SharedWorker so we can't use that as a fallback. |
I just got a report that the crash is still happening even in an iframe. Snap! Edit: It must have been a problem with the |
Google Chrome team have been very responsive and released a series of patches since I first reported this problem (private issue 1372621 for posterity). In my latest round of testing I haven't been able to crash the browser. Woohooo! I'm closing this issue for now. Let's reopen if the problem ever returns. |
Iframe worker threads were introduced as a workaround for limitations in web browsers. Namely: * Chrome crashed when using WASM in web workers * Firefox didn't support ESM workers at all Both problems are now solved: * #1 * mdn/content#26774 There are no more reasons to keep maintaining the iframe worker thread backend. Let's remove it and lean fully on web workers.
Iframe worker threads were introduced as a workaround for limitations in web browsers. Namely: * Chrome crashed when using WASM in web workers * Firefox didn't support ESM workers at all Both problems are now solved: * #1 * mdn/content#26774 There are no more reasons to keep maintaining the iframe worker thread backend. Let's remove it and lean fully on web workers.
Iframe worker threads were introduced as a workaround for limitations in web browsers. Namely: * Chrome crashed when using WASM in web workers * Firefox didn't support ESM workers at all Both problems are now solved: * #1 * mdn/content#26774 There are no more reasons to keep maintaining the iframe worker thread backend. Let's remove it and lean fully on web workers. This commit changes the signature of `spawnPHPWorkerThread` from `spawnPHPWorkerThread(workerUrl, workerType, options)` to `spawnPHPWorkerThread(workerUrl, options)` and is therefore breaking.
Iframe worker threads were introduced as a workaround for limitations in web browsers. Namely: * Chrome crashed when using WASM in web workers * Firefox didn't support ESM workers at all Both problems are now solved: * WordPress/wordpress-playground#1 * mdn/content#26774 There are no more reasons to keep maintaining the iframe worker thread backend. Let's remove it and lean fully on web workers. This commit changes the signature of `spawnPHPWorkerThread` from `spawnPHPWorkerThread(workerUrl, workerType, options)` to `spawnPHPWorkerThread(workerUrl, options)` and is therefore breaking.
## What is this PR doing? Similar to #1357, but taking a deeper cut at it, as import expects to be run within an admin interface. ## What problem is it solving? The following fatal will be triggered: ``` PHP Fatal error: Uncaught Error: Call to undefined function wp_read_audio_metadata() in /wordpress/wp-admin/includes/image.php:2 Stack trace: #0 /wordpress/wp-content/plugins/WordPress-Importer-master/class-wxr-importer.php(1067): wp_generate_attachment_metadata(821, '/wordpress/wp-c...') #1 /wordpress/wp-content/plugins/WordPress-Importer-master/class-wxr-importer.php(861): WXR_Importer->process_attachment(Array, Array, 'https://raw.git...') #2 /wordpress/wp-content/plugins/WordPress-Importer-master/class-wxr-importer.php(383): WXR_Importer->process_post(Array, Array, Array, Array) #3 /internal/eval.php(20): WXR_Importer->import('/tmp/import.wxr') #4 {main} thrown in /wordpress/wp-admin/includes/image.php on line 2 ``` Fixes #1444 (Since found this issue) ## How is the problem addressed? Including includes/admin.php which pulls in all admin-related functionality, rather than just the cropping functionality. Alternatively, this could've been done by selectively importing `includes/media.php` as well as `includes/image.php`, but I anticipate that core is likely to include other related admin functions. ## Testing Instructions Note: This is an untested change. Try running https://playground.wordpress.net/builder/builder.html#{%22preferredVersions%22:{%22php%22:%228.0%22,%22wp%22:%22latest%22},%22phpExtensionBundles%22:[%22kitchen-sink%22],%22features%22:{%22networking%22:true},%22steps%22:[{%22step%22:%22login%22,%22username%22:%22admin%22,%22password%22:%22password%22},{%22step%22:%22importWxr%22,%22file%22:{%22resource%22:%22url%22,%22url%22:%22https://raw.githubusercontent.com/WordPress/theme-test-data/try/cors-accessible-images/themeunittestdata.wordpress.xml%22}}]}
What is this issue about?
The WASM PHP crashes in chrome. It does not crash in Firefox, Safari, and node.js.
See the minimal reproduction in bug-reproduction.zip. It consists of two HTML files: breaks_here.html and works_here.html. The first one demonstrates the problem in the worker and the second one
shows that the issue does not occur in the main threadcrashes too, although less frequently.The issue is the most apparent inside of a webworker, but it also exists when WASM is initialized in the main browser thread. The code below is enough to trigger the crash. Note we don't even run any wasm code, just instantiate the module:
Chromium debugging findings
The Chromium team shared the following stack trace proving this is an out of memory problem:
Other Chromium findings
I did some debugging before they shared that stack trace. The list below is less relevant than the specific details in the stack trace above, but I'm still posting it here for posterity:
TERMINATION_STATUS_PROCESS_CRASHED
.valgrind
would help, but it won't run on Mac.about:crash
and other crashes.This means you need to manually symbolize it to extract any useful information and I didn't get there yet.Older Chromium used breakpad where manual symbolization was needed. Modern Chromium uses crashpad which can be symbolized as follows:Chromium debugging resources
I built Chromium on Mac like this:
Then, I created a new empty xcode project and used the
Debug > Attach to > Chromium
from the top level menu. Finally, I paused the process and set a breakpoint on the error page handler like this:It didn't yield much information so I looked for scraps of information and set further breakpoints:
GetTerminationStatus
– breakpoint wasn't triggeredV8_Fatal
– breakpoint wasn't triggeredPerformShutdownOnWorkerThread
– breakpoint wasn't triggeredSee more information at:
The text was updated successfully, but these errors were encountered: