Skip to content

Fix emrun_file_dump() to work in multithreaded builds. #26698

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

Merged
juj merged 2 commits into from
Apr 17, 2026
+5 −1
Merged

Fix emrun_file_dump() to work in multithreaded builds. #26698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
504c5e8
Fix emrun_file_dump() to work in multithreaded builds.
juj Apr 16, 2026
eb40cf9
Simplify
juj Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/emrun_postjs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ if (globalThis.window && (typeof ENVIRONMENT_IS_PTHREAD == 'undefined' || !ENVIR
var http = new XMLHttpRequest();
out(`Dumping out file "${filename}" with ${data.length} bytes of data.`);
http.open("POST", "stdio.html?file=" + filename, true);
http.send(data); // XXX this does not work in workers, for some odd reason (issue #2681)
if (ArrayBuffer.isView(data) && typeof SharedArrayBuffer !== "undefined" && data.buffer instanceof SharedArrayBuffer) {
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It you look at getUnsharedTextDecoderView I think we can replace this whole expression with just !(data.buffer instanceof ArrayBuffer)

Copy link
Copy Markdown
Collaborator Author

@juj juj Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In emrun_file_dump() hypothetically, data might be a regular array. Since it is invoked from the JS realm (and not C/C++), existing users might have been calling it by feeding a regular array or a typed array, or a typed array view. The comment had

// POSTs the given binary data represented as a (typed) array

so I don't want to change that. That is why the check here is conditional.

data = new data.constructor(data); // Make a clone of the typed array of the same type, since http.send() does not allow SharedArrayBuffer backing.
}
http.send(data);
};

if (globalThis.document) {
Expand Down
1 change: 1 addition & 0 deletions test/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5754,6 +5754,7 @@ def test_program_arg_separator(self):
self.assertContained('error: unrecognized arguments: --foo', err)
self.assertContained('remember to add `--` between arguments', err)

@also_with_threads
def test_emrun(self):
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we are not actually running these tests in CI since they got disabled a while back.

See #26182

We should certain look into fixing that.

Copy link
Copy Markdown
Collaborator Author

@juj juj Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Tending to that is not urgent from my pov at least.

self.emcc('test_emrun.c', ['--emrun', '-o', 'test_emrun.html'])
if not has_browser():
Expand Down
Loading