Skip to content

Don't do memory operations in pthreads before the runtime is initialized. NFC #12052

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
merged 2 commits into from
Aug 28, 2020
Merged

Don't do memory operations in pthreads before the runtime is initialized. NFC #12052

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1364cb1
Don't do memory operations in pthreads before the runtime is initialized
kripken Aug 26, 2020
a9120e6
Merge branch 'master' into prestat
kripken Aug 28, 2020
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
14 changes: 7 additions & 7 deletions src/library_pthread.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@ var LibraryPThread = {
runningWorkers: [],
// Points to a pthread_t structure in the Emscripten main heap, allocated on demand if/when first needed.
// mainThreadBlock: undefined,
initRuntime: function() {
// Pass the thread address inside the asm.js scope to store it for fast access that avoids the need for a FFI out.
// Global constructors trying to access this value will read the wrong value, but that is UB anyway.
registerPthreadPtr(PThread.mainThreadBlock, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
_emscripten_register_main_browser_thread_id(PThread.mainThreadBlock);
},
initMainThreadBlock: function() {
#if ASSERTIONS
assert(!ENVIRONMENT_IS_PTHREAD);
Expand All @@ -41,7 +35,8 @@ var LibraryPThread = {
PThread.allocateUnusedWorker();
}
#endif

},
initRuntime: function() {
PThread.mainThreadBlock = {{{ makeStaticAlloc(C_STRUCTS.pthread.__size__) }}};
Copy link
Collaborator

Choose a reason for hiding this comment

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

After this change, the function initMainThreadBlock no longer initializes PThread.mainThreadBlock (but instead initRuntime does that). That naming mismatch is awkward, so maybe change name initMainThreadBlock to loadPThreadPool or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

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

@juj good point, thanks, I opened #12604 now.


for (var i = 0; i < {{{ C_STRUCTS.pthread.__size__ }}}/4; ++i) HEAPU32[PThread.mainThreadBlock/4+i] = 0;
Expand All @@ -66,6 +61,11 @@ var LibraryPThread = {
PThread.setThreadName(PThread.mainThreadBlock, "Browser main thread");
PThread.setThreadStatus(PThread.mainThreadBlock, {{{ cDefine('EM_THREAD_STATUS_RUNNING') }}});
#endif

// Pass the thread address inside the asm.js scope to store it for fast access that avoids the need for a FFI out.
// Global constructors trying to access this value will read the wrong value, but that is UB anyway.
registerPthreadPtr(PThread.mainThreadBlock, /*isMainBrowserThread=*/!ENVIRONMENT_IS_WORKER, /*isMainRuntimeThread=*/1);
_emscripten_register_main_browser_thread_id(PThread.mainThreadBlock);
},
initWorker: function() {
#if USE_CLOSURE_COMPILER
Expand Down