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

emrun-related JS code is broken #23685

Closed
oitel opened this issue Feb 16, 2025 · 10 comments
Closed

emrun-related JS code is broken #23685

oitel opened this issue Feb 16, 2025 · 10 comments

Comments

@oitel
Copy link
Contributor

oitel commented Feb 16, 2025

emrun cannot get output from a browser (tested for Firefox 135 and Chromium 133) for the apps compiled with emsdk v4.0.3 and v4.0.4:

Web server root directory: /tmp/em_test/v403
Starting web server: http://0.0.0.0:6931/
Starting browser: /usr/bin/xdg-open http://localhost:6931/main.html
/usr/bin/xdg-open
Searching for processes by full path name "/usr/bin/xdg-open".. found 0 entries
Launched browser process with pid=200059
Now listening at http://0.0.0.0:6931/
Entering web server loop.
First navigation occurred. Identifying currently running browser processes
Searching for processes by full path name "/usr/bin/xdg-open".. found 0 entries
Was unable to detect the browser process that was spawned by emrun. This may occur if the target page was opened in a tab on a browser process that already existed before emrun started up.
The html page you are running is not emrun-capable. Stdout, stderr and exit(returncode) capture will not work. Recompile the application with the --emrun linker flag to enable this, or pass --no-emrun-detect to emrun to hide this check.

For the apps compiled with emsdk v4.0.2, emrun works successfully:

Web server root directory: /tmp/em_test/v402
Starting web server: http://0.0.0.0:6931/
Starting browser: /usr/bin/xdg-open http://localhost:6931/main.html
/usr/bin/xdg-open
Searching for processes by full path name "/usr/bin/xdg-open".. found 0 entries
Launched browser process with pid=200392
Now listening at http://0.0.0.0:6931/
Entering web server loop.
First navigation occurred. Identifying currently running browser processes
Searching for processes by full path name "/usr/bin/xdg-open".. found 0 entries
Was unable to detect the browser process that was spawned by emrun. This may occur if the target page was opened in a tab on a browser process that already existed before emrun started up.
POST: "/stdio.html" (path: "/stdio.html", query: "")
POST: "/stdio.html" (path: "/stdio.html", query: "")
Hello, world!
POST: "/stdio.html" (path: "/stdio.html", query: "")
Shutting down because browser is no longer alive
Browser process has shut down, quitting web server.
Web page has quit with a call to exit() with return code 0. Shutting down web server. Pass --serve-after-exit to keep serving even after the page terminates with exit().
Web server loop done.
Closed web server.
emrun quitting with process exit code 0

Seems like it's caused by calling a non-existing addOnExit function from src/emrun_postjs.js. The function was moved in a series of commits:
13947f4 Unify more of the minimal runtime and normal runtime initialization code. (#23488)
a4f64a3 Move all addOn functions into a JS library. (#23565)

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 4.0.3 (a9651ff57165f5710bb09a5fe52590fd6ddb72df)
clang version 21.0.0git (https:/github.com/llvm/llvm-project 6dc41a639334b913e762f65410fcd14a722b137f)
Target: wasm32-unknown-emscripten
Thread model: posix

Failing command line in full:

emcc -s ENVIRONMENT=web --emrun main.cpp -o main.html && emrun --verbose main.html

main.cpp:

#include <iostream>

int main()
{
    std::cout << "Hello, world!" << std::endl;
    return 0;
}

Update 1: added info about v4.0.4.
Update 2: added info about the addOnExit function.
Update 3: updated the title.

@oitel oitel changed the title --emrun flag doesn't work emrun-related JS code is broken Feb 26, 2025
@oitel
Copy link
Contributor Author

oitel commented Mar 4, 2025

Managed to fix it locally by patching the link tool:

diff --git a/tools/link.py b/tools/link.py
index 9440e8908..2e82c0cf4 100644
--- a/tools/link.py
+++ b/tools/link.py
@@ -674,6 +674,8 @@ def phase_linker_setup(options, linker_args):  # noqa: C901, PLR0912, PLR0915
     if user_settings.get('EXIT_RUNTIME') == '0':
       exit_with_error('--emrun is not compatible with EXIT_RUNTIME=0')
     settings.EXIT_RUNTIME = 1
+    # emrun_postjs.js needs this library function.
+    settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$addOnExit']
 
   if options.cpu_profiler:
     options.post_js.append(utils.path_from_root('src/cpuprofiler.js'))

@sbc100
Copy link
Collaborator

sbc100 commented Mar 4, 2025

Thanks so much the fix! Would you like to send a PR. I'll take a look at why that doesn't seem to be covered by our test.

@oitel
Copy link
Contributor Author

oitel commented Mar 4, 2025

Sure thing, I'll prepare a PR a bit later.

sbc100 pushed a commit that referenced this issue Mar 5, 2025
Related to #23685 

This change fixes the runtime error which causes emrun to be unable to
get output from a browser.
@esotericpig
Copy link

Bump. I also get this same error in v4.0.4 of Emscripten with a simple main.cpp:

Uncaught ReferenceError: addOnExit is not defined

@sbc100
Copy link
Collaborator

sbc100 commented Mar 7, 2025

Are you using --emrun @esotericpig?

If not can you see where the undefined reference to addOnExit is coming from? i.e. in devtools can you see a backtrace?

@sbc100
Copy link
Collaborator

sbc100 commented Mar 7, 2025

I believe this specific issue was closes by #23830

@sbc100 sbc100 closed this as completed Mar 7, 2025
@esotericpig
Copy link

Yes, I have --emrun.

I searched here and then compared it to my grep locally:

$ grep --exclude-dir=.git --exclude-dir=cache -ir 'addOnExit'
upstream/emscripten/test/test_other.py:                      '-sEXPORTED_RUNTIME_METHODS=baz,addOnExit',
upstream/emscripten/test/test_other.py:      import init, { _foo as foo, _bar as bar, baz, qux, addOnExit, HEAP32 } from "./modularize_instance.mjs";
upstream/emscripten/test/test_other.py:      assert(typeof addOnExit === 'function'); // exported runtime function with EXPORTED_RUNTIME_METHODS
upstream/emscripten/test/test_other.py:addOnExit(() => console.log("addOnExit"));
upstream/emscripten/test/test_other.py:      ['addOnExit', True],
upstream/emscripten/test/test_other.py:      '-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=$addOnPreRun,$addOnInit,$addOnPostCtor,$addOnPreMain,$addOnExit,$addOnPostRun',
upstream/emscripten/src/threadprofiler.js:    addOnExit(() => clearInterval(i));
upstream/emscripten/src/threadprofiler.js:      addOnExit(() => clearInterval(i));
upstream/emscripten/src/lib/libglut.js:  glutInit__deps: ['$Browser', '$addOnExit'],
upstream/emscripten/src/lib/libglut.js:    addOnExit(() => {
upstream/emscripten/src/lib/libhtml5.js:    '$addOnExit',
upstream/emscripten/src/lib/libhtml5.js:        addOnExit(JSEvents.removeAllEventListeners);
upstream/emscripten/src/lib/libcore.js:  $addOnExit__deps: ['$onExits'],
upstream/emscripten/src/lib/libcore.js:  $addOnExit: (cb) => onExits.unshift(cb),
upstream/emscripten/src/emrun_postjs.js:      addOnExit(() => {
upstream/emscripten/tools/link.py:    settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$addOnInit', '$addOnExit']
upstream/emscripten/ChangeLog.md:   addOnPreRun, addOnInit, addOnPreMain, addOnExit, addOnPostRun,

After comparing the two, I found that link.py only has one instance of 'addOnExit' locally.

My emsdk repo is on branch main. I did git fetch && git pull. I also tried deleting upstream. Then I ran all of the normal stuff:

source emsdk_env.sh
emsdk install latest
emsdk activate latest
source emsdk_env.sh

I did grep again and still not in link.py. Maybe I should delete cache too? Not sure.

Anyway, I manually edited link.py to add this line, and it now works without any errors.

@sbc100
Copy link
Collaborator

sbc100 commented Mar 8, 2025

If you want to try out as-yet-unreleased changes you need to emsdk install tot

@esotericpig
Copy link

If you want to try out as-yet-unreleased changes you need to emsdk install tot

Okay, but using emsdk install latest && emsdk activate latest and building a project causes this error, so I guess that's by design? I thought this would use the latest stable version, but guess it's using the main branch instead?

@sbc100
Copy link
Collaborator

sbc100 commented Mar 10, 2025

If you want to try changes out that have not yet made it into a release you can use emsdk install tot (for Tip-Of-Tree)

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

No branches or pull requests

3 participants