Skip to content
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.

Sign up for GitHub

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

Jump to bottom

getContextSafariWebGL2Fixed create memory leak ? #17409

Open
Jasonlhy opened this issue Jul 11, 2022 · 2 comments
Open

getContextSafariWebGL2Fixed create memory leak ? #17409

Jasonlhy opened this issue Jul 11, 2022 · 2 comments

Comments

@Jasonlhy
Copy link
Contributor

When running emcc with flag -s MODULARIZE=1 -s environment=web, it will generate a commonJS module.
Inside the module function, it stores the wasmMemory and other staff, and it replaces the original canvas.getContext with getContextSafariWebGL2Fixed

        // BUG: Workaround Safari WebGL issue: After successfully acquiring WebGL context on a canvas,
        // calling .getContext() will always return that context independent of which 'webgl' or 'webgl2'
        // context version was passed. See https://bugs.webkit.org/show_bug.cgi?id=222758 and
        // https://github.com/emscripten-core/emscripten/issues/13295.
        // TODO: Once the bug is fixed and shipped in Safari, adjust the Safari version field in above check.
        if (!canvas.getContextSafariWebGL2Fixed) {
          canvas.getContextSafariWebGL2Fixed = canvas.getContext;
          /** @type {function(this:HTMLCanvasElement, string, (Object|null)=): (Object|null)} */
          function fixedGetContext(ver, attrs) {
            var gl = canvas.getContextSafariWebGL2Fixed(ver, attrs);
            return ((ver == 'webgl') == (gl instanceof WebGLRenderingContext)) ? gl : null;
          }
          canvas.getContext = fixedGetContext;
        }

Usually this is not a problem, however, I want to "reload" the module, in order to GC the whole wasmMemory buffer.
For example:

delete require.cache[require.resolve('./mywasm.js')];
var module = require('./mywasm.js')

However, I found out that as long as this function is kept by the canvas element, and the canvas element is kept by the DOM tree, or by some JavaScript (e.g. FiberNode on React), the memory will be kept alive indirectly

Possible solution I found:

  1. Remove the canvas from the DOM tree directly, and make sure no reference to the unattached DOM as well (which is not MVVM framework like React and Angular is designed to do ...)
  2. Delete this monkey patch function manually

Or is there a flag so that this function will not be generated because I don't need to support Safari
@kripken
Copy link
Member

kripken commented Jul 11, 2022

If you don't need to support Safari then you can set -sMIN_SAFARI_VERSION=-1.

Separately, as Safari fixed the issue over a year ago, probably it was released in the latest version, so we could use a version check to skip this automatically when possible.

@Jasonlhy
Copy link
Contributor Author

I just find out that this function can be disabled with flag: GL_WORKAROUND_SAFARI_GETCONTEXT_BUG
https://github.com/emscripten-core/emscripten/blob/fb83ec814d4774a0c50601186cd9aee6e680c59f/src/library_webgl.js

@andyb1979 andyb1979 mentioned this issue Jun 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kripken @Jasonlhy