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.

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

--use-preloaded-plugins fails with pthread enabled #9992

Closed
gabrielcuvillier opened this issue Dec 9, 2019 · 2 comments
Closed

--use-preloaded-plugins fails with pthread enabled #9992

gabrielcuvillier opened this issue Dec 9, 2019 · 2 comments

Comments

@gabrielcuvillier
Copy link
Contributor

Preloaded plugins code is crashing when a program is compiled with -pthread or USE_PTHREADS=1

This bug was found in the context of PR #9947, but is completely unrelated to the Regal port.

Reproducing the issue is fairly simple. For example, in test_browser.py just update the test_glbegin_points with '@requires_threads' facet and '-pthread' compilation flag, like this:

  @requires_threads
  @requires_graphics_hardware
  def test_glbegin_points(self):
    shutil.copyfile(path_from_root('tests', 'screenshot.png'), 'screenshot.png')
    self.btest('glbegin_points.c', reference='glbegin_points.png', args=['--preload-file', 'screenshot.png', '-pthread', '-s', 'LEGACY_GL_EMULATION=1', '-lGL', '-lSDL', '--use-preload-plugins', '-pthread'])

And then run the test (be sure it will run on Chrome as it needs multithreading support):

python tests/runner.py browser.test_glbegin_points

The test will fail silently. If you open the debugger on chrome, you may see the following error that means something start to be wrong at some point:

Blob constructor present but fails: TypeError: Failed to construct 'Blob': The provided ArrayBufferView value must not be shared.; falling back to blob builder"

Some code want a Blob to be constructed out of a Shared Typed Array, and this is apparently not possible (?). So a fallback to a "Blob Builder" is activated. But in turn, the fallback fails.

This happens during the file preload / imagePlugin handlers:

    Uncaught (in promise) TypeError: Browser.BlobBuilder is not a constructor
    at Object.imagePlugin_handle [as handle] (VM15 test.js:1596)
    at VM15 test.js:4359
    at Array.forEach (<anonymous>)
    at processData (VM15 test.js:4356)
    at Object.createPreloadedFile [as FS_createPreloadedFile] (VM15 test.js:4374)
    at DataRequest.finish (VM15 test.js:112)
    at DataRequest.onload (VM15 test.js:108)
    at processPackageData (VM15 test.js:138)
    at runWithFS (VM15 test.js:148)
    at callRuntimeCallbacks (VM15 test.js:1057)

So the methods that fails is 'imagePlugin_handle(byteArray, name, onload, onerror)' in 'library_browser.js', at the following code:

var bb = new Browser.BlobBuilder();

We get there because the initial blob creation failed (due to Shared Typed Array thing). Then, this line fails because 'Browser.BlobBuilder' is null (or undefined, I am not sure). After some investigation, I found out it is so because of this code somewhere earlier in 'library_browser.js':

try {
        new Blob();
        Browser.hasBlobConstructor = true;
      } catch(e) {
        Browser.hasBlobConstructor = false;
        console.log("warning: no blob constructor, cannot create blobs with mimetypes");
      }
Browser.BlobBuilder = typeof MozBlobBuilder != "undefined" ? MozBlobBuilder : (typeof WebKitBlobBuilder != "undefined" ? WebKitBlobBuilder : (!Browser.hasBlobConstructor ? console.log("warning: no BlobBuilder") : null));

Indeed, we are neither on Firefox nor on Webkit, so BlobBuilder is either undefined or null.

From there, I don't know what could be done: unable to create a new Blob because of Shared Typed Arrays + no MozBlobBuilder + no WebKitBlobBuilder = what can be done ?

@kripken
Copy link
Member

kripken commented Dec 9, 2019

Interesting. Based on that error, I think we need to make a copy of the data before sending it to the Blob constructor, as it is apparently defined as not multithreading-safe. I think a copy is fine as we send it once, and don't need to look for updates later, but we should read the code around there to make sure.

(Note that BlobBuilder is only present for backwards compatibility with old browsers - we can probably remove it, or put it behind the legacy flag.)

@stale
Copy link

stale bot commented Dec 9, 2020

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.

@stale stale bot added the wontfix label Dec 9, 2020
@stale stale bot closed this as completed Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants