Fix Windows loop

[balloob]

Description:

Fix the event loop under Windows after #16617.

[awarecan]

HA can start up in Windows Python 3.6.3 now.

However press Ctrl+C cannot stop process, have to kill it through task manager.
First Ctrl+C will output following

Traceback (most recent call last):
  File "c:\awarecan\home-assistant\homeassistant\__main__.py", line 380, in <module>
    sys.exit(main())
  File "c:\awarecan\home-assistant\homeassistant\__main__.py", line 372, in main
    exit_code = asyncio_run(setup_and_run_hass(config_dir, args))
  File "c:\awarecan\home-assistant\homeassistant\util\async_.py", line 29, in asyncio_run
    return loop.run_until_complete(main)
  File "C:\Python\Python36\lib\asyncio\base_events.py", line 454, in run_until_complete
    self.run_forever()
  File "C:\Python\Python36\lib\asyncio\base_events.py", line 421, in run_forever
    self._run_once()
  File "C:\Python\Python36\lib\asyncio\base_events.py", line 1390, in _run_once
    event_list = self._selector.select(timeout)
  File "C:\Python\Python36\lib\selectors.py", line 323, in select
    r, w, _ = self._select(self._readers, self._writers, [], timeout)
  File "C:\Python\Python36\lib\selectors.py", line 314, in _select
    r, w, x = select.select(r, w, w, timeout)
KeyboardInterrupt

The following Ctrl+C didn't got any response

After kill through task manager, get another stack trace output

Traceback (most recent call last):
  File "C:\Python\Python36\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python\Python36\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "c:\awarecan\home-assistant\homeassistant\__main__.py", line 380, in <module>
    sys.exit(main())
  File "c:\awarecan\home-assistant\homeassistant\__main__.py", line 372, in main
    exit_code = asyncio_run(setup_and_run_hass(config_dir, args))
  File "c:\awarecan\home-assistant\homeassistant\util\async_.py", line 29, in asyncio_run
    return loop.run_until_complete(main)
  File "C:\Python\Python36\lib\asyncio\base_events.py", line 454, in run_until_complete
    self.run_forever()
  File "C:\Python\Python36\lib\asyncio\base_events.py", line 421, in run_forever
    self._run_once()
  File "C:\Python\Python36\lib\asyncio\base_events.py", line 1426, in _run_once
    handle._run()
  File "C:\Python\Python36\lib\asyncio\events.py", line 127, in _run
    self._callback(*self._args)
  File "c:\awarecan\home-assistant\homeassistant\__main__.py", line 256, in setup_and_run_hass
    subprocess.check_call(nt_args)
  File "C:\Python\Python36\lib\subprocess.py", line 286, in check_call
    retcode = call(*popenargs, **kwargs)
  File "C:\Python\Python36\lib\subprocess.py", line 269, in call
    return p.wait(timeout=timeout)
  File "C:\Python\Python36\lib\subprocess.py", line 1055, in wait
    timeout_millis)
KeyboardInterrupt

[pvizeli]

Could be simple:

try:
   import uvloop
   asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
except ..:
   pass

[smurfix]

Yes it's simpler, but the part that's supposed to raise an ImportError is the actual import. I don't want to guard the policy call because if uvloop has an internal problem which happens to raise an ImportError the system may be left in an inconsistent state and the error won't be reported.

All of this is probably not relevant in this particular case, but I got into the habit of following this pattern as a matter of principle because it's good coding practice and I did hit that set of circumstances often enough to be wary of it.

[balloob]

CC @smurfix

[smurfix]

See

# Run a simple daemon runner process on Windows to handle restarts

https://github.com/home-assistant/home-assistant/blob/78b6439ee6a07e3aacbd6555dca2595e74d2e8b5/homeassistant/__main__.py#L248

Apparently this runner does no longer get a subprocess.CalledProcessError when hass is killed, probably because it's now in the async part of the code, where it doesn't belong.

Could you check whether the latest change in https://github.com/M-o-a-T/home-assistant/tree/aio_run (git remote add smurfix https://github.com/M-o-a-T/home-assistant; git fetch smurfix; git cherry-pick 02a76f228d58) fixes this problem?

[awarecan]

No change, same error. Even I comment out the whole --runner section, I still cannot use Ctrl+C to shutdown the process, have to kill it by task manager

[smurfix]

Hmm, the other problem seems to be that the signal handler isn't even registered, sorry that I missed that.

See homeassistant/helpers/signal.py -- please change that so that it reads

@callback
@bind_hass
def async_register_signal_handling(hass: HomeAssistant) -> None:
    """Register system signal handler for core."""
    @callback
    def async_signal_handle(exit_code):
        """Wrap signal handling.

        * queue call to shutdown task
        * re-instate default handler
        """
        hass.loop.remove_signal_handler(signal.SIGTERM)
        hass.loop.remove_signal_handler(signal.SIGINT)
        hass.async_create_task(hass.async_stop(exit_code))

    if sys.platform != 'win32':
        try:
            hass.loop.add_signal_handler(
                signal.SIGHUP, async_signal_handle, RESTART_EXIT_CODE)
        except ValueError:
            _LOGGER.warning("Could not bind to SIGHUP")

    try:
        hass.loop.add_signal_handler(
            signal.SIGTERM, async_signal_handle, 0)
    except ValueError:
        _LOGGER.warning("Could not bind to SIGTERM")
    
    try:
        hass.loop.add_signal_handler(
            signal.SIGINT, async_signal_handle, 0)
    except ValueError:
        _LOGGER.warning("Could not bind to SIGINT")

i.e. the SIGINT and SIGTERM parts are no longer guarded by the != win32.

[awarecan]

add_signal_handler() is Unix only

[smurfix]

OK. Use signal.signal then. Sorry about me not having any Python-on-Windows experience …

    if sys.platform != 'win32':
        … original code …
    else:
        old_sigiterm = None
        old_sigint = None

        @callback
        def async_signal_handle(exit_code, frame):
            """Wrap signal handling.

            * queue call to shutdown task
            * re-instate default handler
            """
            signal.signal(signal.SIGTERM, old_sigterm)
            signal.signal(signal.SIGINT, old_sigint)
            hass.async_create_task(hass.async_stop(exit_code))

        try:
            old_sigterm = signal.signal(signal.SIGTERM, async_signal_handle)
        except ValueError:
            _LOGGER.warning("Could not bind to SIGTERM")

        try:
            old_sigint = signal.signal(signal.SIGINT, async_signal_handle)
        except ValueError:
            _LOGGER.warning("Could not bind to SIGINT")

[awarecan]

Looks good this time. I can clean up the change and make a push later

[smurfix]

Phew. ;-) Thanks.

[awarecan]

hass can start and stop on Windows now. but I cannot connect to http://localhost:8123. Investigating...

EDIT: false alarm, I forget I had forward my localhost 8123 port to my dev VM. LOL

[smurfix]

Could you please pull from https://github.com/M-o-a-T/home-assistant/tree/winfix (and push that onto this pull request)?

This change adds the change from comment #16737 to this pull request. Having that code in the async part blocks one of the original goals of moving towards asyncio.run (composeability).

[awarecan]

Restart tested on Windows

[balloob]

Great work everyone