hooiv commented on PR #70163: URL: https://github.com/apache/airflow/pull/70163#issuecomment-5084071241
I did a much deeper dive into the Celery/billiard source code and the Airflow 3 SDK architecture, and I finally have the complete and accurate picture. 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 `billiard` did under the hood. Here is the *actual* sequence of events causing the memory leak/hang: 1. The Celery main process installs a global `SIGCHLD` handler to manage its worker pool using a `waitpid(-1, WNOHANG)` loop. 2. When Celery forks a worker process (via `billiard`), that worker **inherits** the `SIGCHLD` handler. 3. In Airflow 3, the Celery worker receives the `execute_workload` task and directly calls `supervise()`. 4. `supervise()` spawns the Task SDK subprocess via `WatchedSubprocess.start()` (using `os.fork()`). 5. When the Task SDK subprocess finishes, it sends `SIGCHLD` to its parent (the Celery worker). 6. **The Race Condition:** The Celery worker's inherited `billiard` handler intercepts the signal, calls `waitpid(-1)`, and consumes the exit status before the supervisor's `psutil.wait(0)` loop gets a chance to see it! 7. The supervisor loop calls `psutil.wait(0)`, which receives `ECHILD` and gracefully returns `None`. Because our supervisor code didn't handle `None`, it hung infinitely in `epoll_wait()`. I have force-pushed implements a fix: 1. We now explicitly reset `SIGCHLD` to `SIG_DFL` inside `execute_workload()` in `celery_executor_utils.py` before `supervise()` is called. This safely uninstalls the inherited billiard handler for the duration of the workload execution. (Note: My previous commit put this reset in `_execute_in_fork`, which is only used by Airflow 2, completely missing the Airflow 3 execution path). 2. I modified `_check_subprocess_exit` in `supervisor.py` to handle `psutil.wait()` returning `None` by setting the exit code to `-1` and returning immediately. This is a critical safety net that ensures the supervisor will never hang if an exit status is lost (e.g. if the supervisor crashes and the child actually does reparent to `dumb-init`). I have also added a dedicated unit test in `test_supervisor.py` to cover the `None` scenario. -- 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]
