Fix memory leak in Chrome - Error objects do not like to be persistent #9108
Conversation
This file contains 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
…t, they seem to heavily capture variables they have in their scope.
kripken
approved these changes
Jul 29, 2019
|
Do you think this is related to #8571, or the linked Chrome bugs? |
|
It is possible, although it's hard to say without profiling - definitely worth a retest after this lands. |
belraquib
pushed a commit
to belraquib/emscripten
that referenced
this pull request
Dec 23, 2020
emscripten-core#9108) * Fix memory leak in Chrome - Error objects do not like to be persistent, they seem to heavily capture variables they have in their scope. * Add comment in ChangeLog
adamziel
added a commit
to WordPress/wordpress-playground
that referenced
this pull request
May 30, 2023
…race attached Overrides Emscripten's default ExitStatus object which gets thrown on failure. Unfortunately, the default object is not a subclass of Error and does not provide any stack trace. This is a deliberate behavior on Emscripten's end to prevent memory leaks after the program exits. See: emscripten-core/emscripten#9108 In case of WordPress Playground, the worker in which the PHP runs will typically exit after the PHP program finishes, so we don't have to worry about memory leaks. As for assigning to a previously undeclared ExitStatus variable here, the Emscripten module declares `ExitStatus` as `function ExitStatus` which means it gets hoisted to the top of the scope and can be reassigned here – before the actual declaration is reached. If that sounds weird, try this example: ```js ExitStatus = () => { console.log("reassigned"); } function ExitStatus() {} ExitStatus(); // logs "reassigned" ```
adamziel
added a commit
to WordPress/wordpress-playground
that referenced
this pull request
May 30, 2023
…race attached (#470) ## Description Overrides Emscripten's default ExitStatus object which gets thrown on failure. Unfortunately, the default object is not a subclass of Error and does not provide any stack trace. This is a deliberate behavior on Emscripten's end to prevent memory leaks after the program exits. See: emscripten-core/emscripten#9108 In case of WordPress Playground, the worker in which the PHP runs will typically exit after the PHP program finishes, so we don't have to worry about memory leaks. As for assigning to a previously undeclared ExitStatus variable here, the Emscripten module declares `ExitStatus` as `function ExitStatus` which means it gets hoisted to the top of the scope and can be reassigned here – before the actual declaration is reached. If that sounds weird, try this example: ```js ExitStatus = () => { console.log("reassigned"); } function ExitStatus() {} ExitStatus(); // logs "reassigned" ``` ## Testing instructions Confirm the CI tests passed Related: #416 cc @wojtekn
Pookie717
added a commit
to Pookie717/wordpress-playground
that referenced
this pull request
Oct 1, 2023
…race attached (#470) ## Description Overrides Emscripten's default ExitStatus object which gets thrown on failure. Unfortunately, the default object is not a subclass of Error and does not provide any stack trace. This is a deliberate behavior on Emscripten's end to prevent memory leaks after the program exits. See: emscripten-core/emscripten#9108 In case of WordPress Playground, the worker in which the PHP runs will typically exit after the PHP program finishes, so we don't have to worry about memory leaks. As for assigning to a previously undeclared ExitStatus variable here, the Emscripten module declares `ExitStatus` as `function ExitStatus` which means it gets hoisted to the top of the scope and can be reassigned here – before the actual declaration is reached. If that sounds weird, try this example: ```js ExitStatus = () => { console.log("reassigned"); } function ExitStatus() {} ExitStatus(); // logs "reassigned" ``` ## Testing instructions Confirm the CI tests passed Related: WordPress/wordpress-playground#416 cc @wojtekn
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.
Fix memory leak in Chrome - Error objects do not like to be persistent, they seem to heavily capture variables they have in their scope.
In Chrome, calling
var x = new Error();in global scope causes an immediate capture of the scope, which leads inModulebeing captured, along with.data,.wasm,.memetc. raw byte arrays, never reclaiming the memory.I don't think there's any pressing reason for ExitStatus to inherit from Error anyways? (was it due to convenience to get a pretty-printed error callstack?)