Emscripten: `-s MODULARIZE=1` does not work with pthreads

[surma]

-s MODULARIZE=1 doesn’t seem to work with pthreads.

It compiles with no warning or errors, it loads and spawns workers. But and then pthread-main.js starts throwing errors. The compiler should catch this.

[kripken]

Thanks @surma, after looking into this, the issue is that pthread-main.js (the script new Worker is called with) overrides things in the global namespace before loading the main JS, such as ENVIRONMENT_IS_PTHREAD. In modularize mode the main JS is in a function scope, so those are not picked up.

Opened #7450 to show an error for that.

Fixing this should not be too hard, we should move the customizations in pthread-main.js into the normal JS, and leave pthread-main.js with just the loading of the main JS, with Module.inPthread or something like that set to true, so that the normal JS knows to apply those customizations.

[juj]

Oh, I actually have support for this implemented in my fork of Emscripten - looking to submit a PR about this once the other multithreading patches have landed - the implementation builds on top of them.

[brion]

@juj awesome! Will test your fork on my ogv.js builds in a bit (or should I wait for the pr?)

[jakogut]

@juj Which branch implements this? I'd be happy to give it a test.

[kripken]

@juj - is that code ready to be submitted now? I think all the other PRs have landed.

[juj]

The implementation is in branch https://github.com/juj/emscripten/tree/multithreading_july_2018, see https://github.com/kripken/emscripten/compare/incoming...juj:multithreading_july_2018 . I have not had time yet to make it ready to be submitted.

[briangavin]

Any progress on this issue?

[VirtualTim]

@juj What's going on with this issue? Is there any eta on when your patch should be merged?

[juj]

@juj What's going on with this issue? Is there any eta on when your patch should be merged?

You can follow the patch PR at #7667.

[niklasf]

@juj Thank you very much for all your work here.

When creating a second instance of a module it fails with Uncaught TypeError: Cannot set property '_stdin' of undefined near code like:

        var _stdin;
        if (ENVIRONMENT_IS_PTHREAD)
            _stdin = PthreadWorkerInit._stdin;
        else
            PthreadWorkerInit._stdin = _stdin = 1036288;

Is there still some shared global state?

[brion]

@niklasf I've got a test case for that on https://github.com/emscripten-core/emscripten/pull/8035 -- the ENVIRONMENT_IS_PTHREAD var leaks into global state, causing some of the logic to get confused on the second run.

Should get fixed up soon; as a workaround, you can run delete window.ENVIRONMENT_IS_PTHREAD before your second instantiation.

[niklasf]

Looks like this has been fixed, possibly via https://github.com/emscripten-core/emscripten/pull/8306.

[kripken]

Thanks @niklasf - yes, this has been fixed. May have been https://github.com/emscripten-core/emscripten/pull/7667 too.