Copy the wasm exports object if we intend to modify it

[kripken]

That makes it work if it is the ES6 exports object of a wasm module, which is read-only.

Fixes #8678 and also bysyncify in ES6 environments.

@VirtualTim - thoughts? Also, I am having a hard time writing a testcase for this - I'm not really familiar with ES6 modules I guess, so I haven't added a test or verified this works yet. Help would be welcome :)

[VirtualTim]

originalexports should have capital E

[VirtualTim]

Sorry, doesn't fix the issue for me. I still get Cannot assign to read only property '___cxa_can_catch' of object '[object Object]'
and the generated js still outputs:

var real____cxa_can_catch = asm["___cxa_can_catch"];
asm["___cxa_can_catch"] = function() {
  assert(runtimeInitialized, 'you need to wait for the runtime to be ready (e.g. wait for main() to be called)');
  assert(!runtimeExited, 'the runtime was exited (use NO_EXIT_RUNTIME to keep it alive after main() exits)');
  return real____cxa_can_catch.apply(null, arguments);
};
[...]
var ___cxa_can_catch = Module["___cxa_can_catch"] = function() {
  assert(runtimeInitialized, 'you need to wait for the runtime to be ready (e.g. wait for main() to be called)');
  assert(!runtimeExited, 'the runtime was exited (use NO_EXIT_RUNTIME to keep it alive after main() exits)');
  return Module["asm"]["___cxa_can_catch"].apply(null, arguments)
};

I am still compiling with the old backend (WASM_OBJECT_FILES=0), so I don't know if this changes anything.

[kripken]

Thanks for the comments, I fixed those issues and tried to add a test.

But the test is still not valid, as it passes even without this fix. @VirtualTim you say you still see the bug - is the test not written properly to catch what you are doing to hit it?

[kripken]

(which backend is used should not affect this, I don't think, and I see the test passes in both backends)

[kripken]

This isn't related to the real issue, but the tests pass locally for me but fail on CI. It's like on CI there is no support for es6 modules, even though based on caniuse there should be. I guess we need a flag or something?

[VirtualTim]

@VirtualTim you say you still see the bug - is the test not written properly to catch what you are doing to hit it?

I figured out the problem with the test, the issue is only there when a thread/webworker is created, so you'll need to add -s PTHREAD_POOL_SIZE=1 to see it.

[VirtualTim]

Unfortunately this has revealed that MODULARIZE_INSTANCE isn't compatible with EXPORT_ES6 + USE_PTHREADS. The first bit is that is uses document.currentScript.src, which is undefined in the worker. I've added the fix for that to #8940, since I was changing that anyway.

After fixing that I'm still getting {{{ EXPORT_NAME }}}.default is not a function from here:

emscripten/src/worker.js

Line 140 in 16edbe5

Module = {{{ EXPORT_NAME }}}.default(Module);

That one is also easy to fix, that line just shouldn't be there in MODULARIZE_INSTANCE.

However the biggest issue is that Module overrides don't appear to work in MODULARIZE_INSTANCE.
In this specific case I'm defining things like Module['ENVIRONMENT_IS_PTHREAD'] = true;, then calling Module = {{{ EXPORT_NAME }}}.default(Module); . However since MODULARIZE_INSTANCE initialises itself that line shouldn't be there, and so no overrides can be basses into the module. I think this is an overall MODULARIZE_INSTANCE + USE_PTHREADS issue.
If you look at the compiled output_name.worker.js you'll see a bunch of Module overrides just before the import(e.data.urlOrBlob) call (eg Module['STACK_MAX'] = Module['STACKTOP'] = 0x7FFFFFFF;). I don't know how that can work in MODULARIZE_INSTANCE.

I hope that all makes sense?

[VirtualTim]

This should work on Chrome if --enable-experimental-web-platform-features is set (sorry, forgot to mention that).
The Firefox bug is also: https://bugzilla.mozilla.org/show_bug.cgi?id=1540913

[sbc100]

The deletion of the incoming branch triggered the automatic closing of this branch. My apologies. This was an unexpected side effect.

Re-opening and re-targeting to master.

[stale]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

[kleisauke]

It looks like this PR has been superseded by #8926. Although, the Firefox bug mentioned by @VirtualTim is still not fixed and prevents the -sEXPORT_ES6 + -pthread combo from working.

Should I open a new issue to track that? Not sure if we should track Firefox issues, but maybe we should document that somehow. FWIW, the wasm-vips playground doesn't currently work on Firefox due to this. Edit: fixed with commit kleisauke/wasm-vips@ba6f7ee.

[VirtualTim]

@kleisauke if you're interested in getting this working in Firefox, I managed to get my use case working by using a custom build of Shimport 1.0.1 containing the fix mentioned here.

[stale]

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.