Skip to content

Don't walk the main module's table in pthread workers - #28105

Open
clementperon wants to merge 5 commits into
emscripten-core:mainfrom
clementperon:dylink-worker-export-slots
Open

clementperon wants to merge 5 commits into
emscripten-core:mainfrom
clementperon:dylink-worker-export-slots

Conversation

@clementperon

@clementperon clementperon commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

The first call to getFunctionAddress walks the whole table and keeps a JS function object for every entry. With MAIN_MODULE and pthreads, every worker makes that call when it instantiates the main module (updateGOT), so each worker holds a wrapper for every address-taken function of the main module for its whole lifetime.

The dynamic linker only looks up the main module's exported functions. So the main thread now records their table slots, and passes them to workers in the load message next to sharedModules. Right before updateGOT, a worker creates functionsInTableMap with just those slots. At that point the table only holds the main module. Side modules add their own slots when they are loaded, as before.

Both halves are in one function, initFunctionsInTableMap in libdylink.js. addFunction and getFunctionAddress are unchanged.

Impact

Kodi's WebAssembly port links Kodi as MAIN_MODULE=2 so binary add-ons can be dlopened. Its table has 50,551 slots, of which 102 hold exported functions. Measured in Chrome with the same wasm, applying this change to the Emscripten 6.0.9 glue:

before after
JS heap per worker, after an add-on is loaded 15.2 MiB 7.2 MiB
JS heap of 17 workers during playback 257 MiB 117 MiB

Behavior

Lookups by the dynamic linker in workers return the same slots as before. A function that appears in several slots still resolves to its last slot, as the walk did.

The difference is a non-exported main-module function passed to addFunction in a worker. JS can only reach such a function through the table itself (for example with getWasmTableEntry), so it already has its address. Before, the walk found its existing slot. Now, the worker gives it a new slot, so it has a different address there than on the main thread. Keeping the old behavior would need the full walk this PR removes. With ASSERTIONS=2, addFunction already reports this case ("function in Table but not functionsInTableMap"). The limitation is noted in a comment on initFunctionsInTableMap and in the ChangeLog entry.

The main thread still walks the table once. #27993 creates the main module's GOT entries lazily and doesn't change the walk, so the two changes are independent.

Testing

test_pthread_dylink now asserts that a side module sees the same &bar_main as the main module, on the main thread and in a worker. That assertion fails if workers don't add the recorded slots.

Passing: all test_pthread_dl*, test_pthread_dylink* and test_pthread_tls_dylink tests in core0, core2 and wasm64, other.test_dylink_pthread*, and the dylink codesize tests (unchanged).

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request optimizes worker initialization in pthreads builds with MAIN_MODULE by passing only the table slots of the main module's exported functions to workers, rather than having them walk the entire function table. Feedback suggests adding a defensive check in src/lib/libaddfunction.js to ensure getWasmTableEntry(slot) returns a valid object before adding it to the WeakMap, preventing potential runtime crashes in workers.

Comment thread src/lib/libaddfunction.js Resolved
@clementperon

Copy link
Copy Markdown
Contributor Author

@sbc100 is this something you want to merge upstream ?

@sbc100

sbc100 commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Mostly likely yes, I'm just a little review constrained right now.

I'll try to get to it.

@clementperon

Copy link
Copy Markdown
Contributor Author

Thank you, take your time and don't hesitate if you want me to perform more test.

Comment thread src/lib/libaddfunction.js Outdated
// As a consequence, a main-module function that is in the table but not
// exported is not found here, so addFunction in a worker gives it a new
// slot rather than the address it has on the main thread (ASSERTIONS=2
// reports this in addFunction).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I'm reading this right it sounds like this means that an address might not represent the same code on all threads? I don't think that can ever work since function pointer can be passed via linear memory and must be callable from all threads.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree I can fix it by having workers scan the main module's table slots once, the first time addFunction misses after startup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@sbc100 I have revert it as in practice it never happens.

User code needs this exact pattern to hit it:

  1. Get a C function pointer to a non-exported function.
  2. In JS, turn it back into a function object with getWasmTableEntry(ptr).
  3. In a worker, pass that object to addFunction to get a pointer again.
  4. Compare that pointer with one from another thread, or rely on it being the same.

@clementperon
clementperon force-pushed the dylink-worker-export-slots branch 3 times, most recently from 509af0f to 12b8416 Compare November 3, 2026 10:19
clementperon and others added 5 commits November 8, 2026 11:24
The first call to getFunctionAddress walks the whole table, which creates a
JS function object for every entry.  With MAIN_MODULE and pthreads, every
worker does this when it instantiates the main module, and keeps those
objects for its lifetime: about 8 MiB per worker for a 50k-entry table.

The dynamic linker only looks up the main module's exported functions, so
the main thread now records their table slots and passes them to workers,
which add just those to the map.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Note in getFunctionAddress and the ChangeLog that a non-exported
main-module function passed to addFunction in a worker now gets a new
slot. Add an ASSERTIONS=2 variant of test_pthread_dylink, whose
addFunction check catches export slots missing from a worker's map.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Instead of special-casing workers in getFunctionAddress, fill their
functionsInTableMap with the main module's export slots right before
updateGOT.  At that point the table only holds the main module, and side
modules add their own slots when they are loaded, so workers no longer
need the main module's table size.  The main thread's recording and the
worker's seeding are now one function, initFunctionsInTableMap.

Drop the ASSERTIONS=2 variant of test_pthread_dylink: the &bar_main
assertion already fails if workers don't get the export slots.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@clementperon
clementperon force-pushed the dylink-worker-export-slots branch from 12b8416 to 0b49138 Compare November 8, 2026 10:27

This branch has not been deployed

No deployments
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.

2 participants