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

Windows: emrun.py does not exit as expected #22578

Open
Markus87 opened this issue Aug 20, 2021 · 3 comments · May be fixed by #22577
Open

Windows: emrun.py does not exit as expected #22578

Markus87 opened this issue Aug 20, 2021 · 3 comments · May be fixed by #22577

Comments

@Markus87
Copy link
Contributor

After the program in the browser finished emrun just hangs at: httpd.server_close()

Command:

emrun --verbose --browser=chrome --browser_args="--headless --disable-gpu --remote-debugging-port=9222" --kill_exit SomeTest.html -- --run_test=* --log_level=test_suite

Output:

Web server root directory: D:\BitFactory\Alex\wasm\Bin\Emscripten_RelWithDebInfo
Starting web server: http://0.0.0.0:6931/
Starting browser: C:\Program Files (x86)\Google\Chrome\Application\chrome.exe --headless --disable-gpu --remote-debugging-port=9222 --enable-nacl --enable-pnacl --disable-restore-session-state --enable-webgl --no-default-browser-check --no-first-run --allow-file-access-from-files http://localhost:6931/Bfx.Alex.UnitTest.ComputationOutputStateHasError.html?--run_test=*&--log_level=test_suite
C:\Program Files (x86)\Google\Chrome\Application\chrome.exe
import psutil failed, unable to detect browser processes
Searching for processes by full path name "C:\Program Files (x86)\Google\Chrome\Application\chrome.exe".. found 0 entries
Launched browser process with pid=17636
Entering web server loop.
First navigation occurred. Identifying currently running browser processes
import psutil failed, unable to detect browser processes
Searching for processes by full path name "C:\Program Files (x86)\Google\Chrome\Application\chrome.exe".. 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.
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.

Possible fix: (Disclaimer: I have no python skills)

  def serve_forever(self, timeout=0.5):
    global last_message_time, page_exit_code, emrun_not_enabled_nag_printed
    socketserver.ThreadingMixIn.block_on_close = False    #+ fixes it for me
    self.is_running = True

I am not sure if this is a problem on operating systems other than Windows.
It also looks like the python deployed with emsdk is missing a module 'psutil'. I guess this would cause --kill_start to not work.

@sbc100
Copy link
Collaborator

sbc100 commented Aug 20, 2021

We could as psutil to the emsdk python if that would fix the issue?

@Markus87
Copy link
Contributor Author

Markus87 commented Aug 20, 2021

I think the missing module has nothing to do with the --kill_exit but the --kill_start functionality of the script.
(I checked the script again, it may be used for many more things, there is a lot of globals involved 😬, my python knowledge is at this point a few hours old)

--kill_exit does not work because the script hangs on httpd.server_close().
This may have to do with a change of behaviour in python 3.7.
As stated here: https://docs.python.org/3/library/socketserver.html

Changed in version 3.7: socketserver.ForkingMixIn.server_close() and socketserver.ThreadingMixIn.server_close() now waits until all child processes and non-daemonic threads complete. Add a new socketserver.ForkingMixIn.block_on_close class attribute to opt-in for the pre-3.7 behaviour.

I dont know why a thread prevents the server_close() from finishing. (maybe a connection held by the browser?)

Sorry about mentioning two problems in one issue, I should have made a separate issue for the last sentence.
I have not tried yet if --kill_start works but I would not expect it too without psutil because the module is used to find instances of running browers.
After I have tested if it works I can also make a new issue for this if you prefer that, I cant try if it would work with the module because I dont know how to install a module to the emsdk python. (there is no pip?)

@rghosh0
Copy link

rghosh0 commented Sep 17, 2024

Hi, we observe the same issue on Windows: emrun will often hang after the server shuts down. We noticed that, as suggested above, setting the server's block_on_close to False sometimes helped,

https://docs.python.org/3/library/socketserver.html

ThreadingMixIn.server_close waits until all non-daemon threads complete, except if block_on_close attribute is False.

but sometimes the program would still block on sys.exit(...). We could then also replace that with os._exit(...) instead

https://stackoverflow.com/questions/19747371/python-exit-commands-why-so-many-and-when-should-each-be-used

os._exit() exits the program without calling cleanup handlers, flushing stdio buffers, etc. Thus, it is not a standard way to exit and should only be used in special cases.

And this combination then worked reliably but seems dirty. I think the reason sys.exit would hang if block_on_close were set to False is that then the server's daemon threads are not joined on close

https://github.com/python/cpython/blob/main/Lib/socketserver.py#L673C7-L673C21

So we found a (comparatively) cleaner solution to just set the server's socket itself to non-blocking before shutdown. This way the threads can still be joined and the ordinary and cleaner sys.exit(...) exit function can be used. Here is a diff that we propose to be adopted (which makes this behaviour a program option for emrun).

diff --git a/emrun.py b/emrun.py
index f7bc13df3..24c1a518a 100644
--- a/emrun.py
+++ b/emrun.py
@@ -704,6 +704,8 @@ class HTTPHandler(SimpleHTTPRequestHandler):
         if not emrun_options.serve_after_exit:
           page_exit_code = int(data[6:])
           logv('Web page has quit with a call to exit() with return code ' + str(page_exit_code) + '. Shutting down web server. Pass --serve-after-exit to keep serving even after the page terminates with exit().')
+          if emrun_options.force_exit:
+            self.server.socket.setblocking(False)
           self.server.shutdown()
           return
       else:
@@ -1563,6 +1565,9 @@ def parse_args(args):
   parser.add_argument('--dump-out-directory', default='dump_out', type=str,
                       help='If specified, overrides the directory for dump files using emrun_file_dump method.')

+  parser.add_argument('--force-exit', action='store_true',
+                      help='If true, sets server socket to nonblocking on shutdown to avoid sporadic deadlocks.')
+
   parser.add_argument('serve', nargs='?', default='')

   parser.add_argument('cmdlineparams', nargs='*')

Pull request: #22577

@rghosh0 rghosh0 linked a pull request Sep 17, 2024 that will close this issue
@sbc100 sbc100 transferred this issue from emscripten-core/emsdk Sep 17, 2024
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 a pull request may close this issue.

3 participants