This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new ac00547512 Replace daemon Thread in dev-mode of breeze start-airflow
with forking (#31403)
ac00547512 is described below
commit ac00547512f33b1222d699c7857108360d99b233
Author: Jarek Potiuk <[email protected]>
AuthorDate: Fri May 19 11:58:16 2023 +0200
Replace daemon Thread in dev-mode of breeze start-airflow with forking
(#31403)
* Replace daemon Thread in dev-mode of breeze start-airflow with forking
When you run the dev mode for Breeze start-airlfow the webpack to
compile the assets is run in the background and should be stopped
as soon as the main process leaves. This has been done via daemon
Thread, however it turns out that when daemon thread gets stopped
at exit, it is stopped pretty abruptly. Quoting
https://docs.python.org/3/library/threading.html ::
> Note Daemon threads are abruptly stopped at shutdown. Their resources
(such as open files, database transactions, etc.) may not be released
properly. If you want your threads to stop gracefully, make them
non-daemonic and use a suitable signalling mechanism such as an Event.
This also means that if the daemon threds starts a subprocess, the
thread is killed but the subprocess is not - the subprocess gets
orphaned and gets adopted by init process and continues running.
Tis PR replaces the mechanism with forking. Instead of running
thread, it will fork a new process that will run the compilation
(including starting the subprocess that runs it). It will also
change group for the started forked process to be the same as
the new pid, which creates a new process group and register
an atexit method to send TERM to all the processes belonging to
that grop, so even if the node process that runs compilation does
not propagate the TERM signal (it does not) to the webpack processes,
all the webpack processes share the same process group and get the
SIGTERM signal.
Fixes: #31384
* Update dev/breeze/src/airflow_breeze/utils/run_utils.py
Co-authored-by: Pierre Jeambrun <[email protected]>
---------
Co-authored-by: Pierre Jeambrun <[email protected]>
---
dev/breeze/src/airflow_breeze/utils/run_utils.py | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/dev/breeze/src/airflow_breeze/utils/run_utils.py
b/dev/breeze/src/airflow_breeze/utils/run_utils.py
index 66bc3e1a40..f29ca43006 100644
--- a/dev/breeze/src/airflow_breeze/utils/run_utils.py
+++ b/dev/breeze/src/airflow_breeze/utils/run_utils.py
@@ -17,17 +17,18 @@
"""Useful tools for running commands."""
from __future__ import annotations
+import atexit
import contextlib
import os
import re
import shlex
+import signal
import stat
import subprocess
import sys
from distutils.version import StrictVersion
from functools import lru_cache
from pathlib import Path
-from threading import Thread
from typing import Mapping, Union
from rich.markup import escape
@@ -470,7 +471,15 @@ def run_compile_www_assets(
f"{WWW_ASSET_OUT_DEV_MODE_FILE if dev else WWW_ASSET_OUT_FILE}\n"
)
if run_in_background:
- thread = Thread(daemon=True, target=_run_compile_internally,
args=(command_to_execute, dev))
- thread.start()
+ pid = os.fork()
+ if pid:
+ # Parent process - send signal to process group of the child
process
+ atexit.register(os.killpg, pid, signal.SIGTERM)
+ else:
+ # Check if we are not a group leader already (We should not be)
+ if os.getpid() != os.getsid(0):
+ # and create a new process group where we are the leader
+ os.setpgid(0, 0)
+ _run_compile_internally(command_to_execute, dev)
else:
return _run_compile_internally(command_to_execute, dev)
