hooiv commented on PR #70163: URL: https://github.com/apache/airflow/pull/70163#issuecomment-5083954600
My sincere apologies! I led myself completely astray in my previous comment. I did a deep dive into the Celery/billiard source code today and realized my assumption about illiard was **completely wrong**. Billiard does **not** install a global SIGCHLD handler that blindly reaps arbitrary child processes with waitpid(-1). Instead, it tracks specific PIDs. The reproducer I provided earlier was flawed because I explicitly injected a global handler into the script to force the failure, incorrectly assuming that's what illiard did under the hood. The original diagnosis in #70117 was entirely correct: **dumb-init** (or whichever process is running as PID 1 in the container) is the real culprit. dumb-init is designed to aggressively reap orphaned child processes using waitpid(-1). When the Airflow task subprocess exits, dumb-init races the Airflow Task SDK supervisor. If dumb-init wins and reaps the child, the OS discards the child's exit status. When our supervisor subsequently calls psutil.wait(timeout=0), the underlying OS call os.waitpid(pid, WNOHANG) raises ECHILD (No child processes). psutil catches this ChildProcessError and, realizing the process no longer exists, gracefully returns None. In our supervisor loop (_check_subprocess_exit), we were not handling this None return value. As a result, self._exit_code remained None, meaning the loop in _monitor_subprocess never satisfied the condition if self._exit_code is not None, preventing SOCKET_CLEANUP_TIMEOUT from ever triggering. I have completely reverted my misguided SIGCHLD changes and restored the correct fix: when psutil.wait() returns None, we now explicitly map it to an exit code of -1 (and log a warning that the exit status was consumed externally). This allows the supervisor to exit its loop gracefully. I have also added a dedicated unit test in est_supervisor.py to cover this exact scenario. Again, I apologize for the confusion I caused with my previous explanation! The branch has been force-pushed with the correct fix and test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
