apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
37.26k stars 14.33k forks source link

Ensure that Task SDK supervisor closes all its subprocess handles correctly. #44263

Closed ashb closed 2 hours ago

ashb commented 18 hours ago

If we keep any copies of the handles open then the selector loop in monitor process will get stuck waiting on a read loop on a socket that never closes (because its open in the same process still!).

Sadly I wasn't able to reproduce this behaviour in unit tests, but only when running this code for real. (The main fix here is to pass child_stderr to _close_unused_sockets too, the rest are drive-by tidy ups)

Since we never access proc.stdout or stderr on the class (they are closed over in the callbacks only) I removed those properties as they aren't needed and shouldn't be accessed directly as it would lead to garbled output.

Also add tests (and re-fix) the last-chance exception handling


^ Add meaningful description above Read the Pull Request Guidelines for more information. In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed. In case of a new dependency, check compliance with the ASF 3rd Party License Policy. In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.