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

Re-enable extern-pre-js and extern-post-js under pthreads #22013

Merged
merged 1 commit into from
May 30, 2024

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented May 29, 2024

In #21701 the running of extern-pre/post-js in pthreads was disabled. The reason was that we had a couple of tests that seemed to be assuming this. However, it turns out that there are important use cases that depend on this code running in threads.

Instead, fix the two test cases by adding an extra condition.

Fixes: #22012

@sbc100 sbc100 requested a review from kripken May 29, 2024 22:33
In emscripten-core#21701 the running of extern-pre/post-js in pthreads was disabled.
The reason was that we had a couple of tests that seemed to be assuming
this.  However, it turns out that there are important use cases that
depend on this code running in threads.

Instead, fix the two test cases by adding an extra condition.

Fixes: emscripten-core#22012
@sbc100 sbc100 requested a review from dschuff May 30, 2024 15:29
@sbc100 sbc100 enabled auto-merge (squash) May 30, 2024 15:41
@sbc100 sbc100 merged commit 845ad34 into emscripten-core:main May 30, 2024
29 checks passed
@sbc100 sbc100 deleted the extern_post_js branch May 30, 2024 15:58
msorvig pushed a commit to msorvig/emscripten that referenced this pull request Jun 4, 2024
…-core#22013)

In emscripten-core#21701 the running of extern-pre/post-js in pthreads was disabled.
The reason was that we had a couple of tests that seemed to be assuming
this.  However, it turns out that there are important use cases that
depend on this code running in threads.

Instead, fix the two test cases by adding an extra condition.

Fixes: emscripten-core#22012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--extern-pre-js no longer accessible in Workers
2 participants