Skip to content

Do not leak threads from InProcessExecutionAPI - #68840

Merged
ashb merged 1 commit into
apache:mainfrom
nickstenning:fix-thread-leak
Jun 22, 2026
Merged

ashb merged 1 commit into
apache:mainfrom
nickstenning:fix-thread-leak

Conversation

@nickstenning

@nickstenning nickstenning commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor

Through a2wsgi.ASGIMiddleware, the InProcessExecutionAPI was leaking a daemon thread per instance, used by a2wsgi to run an asyncio event loop.

This commit moves management of that background thread into InProcessExecutionAPI, and adds a finalizer so that the thread is stopped when the execution API instance is garbage collected.

Note: This also changes how we execute the FastAPI app's lifecycle hooks so that they are required to complete before the transport property returns a value. I think this is probably the correct behaviour anyway, but the change is in this PR to make it simpler to run the CM close hook on teardown.

Through `a2wsgi.ASGIMiddleware`, the `InProcessExecutionAPI` was leaking
a daemon thread per instance, used by `a2wsgi` to run an asyncio event
loop.

This commit moves management of that background thread into
`InProcessExecutionAPI`, and adds a finalizer so that the thread is
stopped when the execution API instance is garbage collected.
@boring-cyborg boring-cyborg Bot added area:API Airflow's REST/HTTP API area:task-sdk labels Jun 22, 2026

@amoghrajesh amoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ashb
ashb merged commit a52be75 into apache:main Jun 22, 2026
77 checks passed
ephraimbuddy pushed a commit to astronomer/airflow that referenced this pull request Jun 22, 2026
Through `a2wsgi.ASGIMiddleware`, the `InProcessExecutionAPI` was leaking
a daemon thread per instance, used by `a2wsgi` to run an asyncio event
loop.

This commit moves management of that background thread into
`InProcessExecutionAPI`, and adds a finalizer so that the thread is
stopped when the execution API instance is garbage collected.
o-nikolas pushed a commit that referenced this pull request Jun 23, 2026
#68865)

* Fix in-process Execution API loop stopped while transport still in use

#68840 moved the InProcessExecutionAPI background event-loop + thread
cleanup into a weakref.finalize keyed on the InProcessExecutionAPI
instance. But callers build a sync Client from
InProcessExecutionAPI().transport and discard the factory object, so the
instance is garbage-collected while the transport is still in use. The
finalizer then stops the loop, and every subsequent request hangs on the
dead loop -- surfacing as Timeout failures across the Dag-processor and
triggerer in-process API tests on main (Error while closing in-process
execution API lifespan -> TimeoutError).

Key the finalizer on the returned WSGI transport instead of the factory
instance, so loop/thread/lifespan teardown happens when the transport
(which the Client holds) is collected, not when the throwaway factory is.

Verified locally: test_processor.py::test_top_level_variable_set and
test_top_level_variable_access_not_found now pass (previously hung to
timeout).

* Test transport-tied lifecycle for in-process Execution API

Rewrite test_in_process_execution_api_teardown as
test_in_process_execution_api_transport_lifecycle: assert that dropping
the factory instance leaves the loop running while the transport is held,
and only dropping the transport stops the loop + joins the daemon thread.
This fails against finalizing on the instance (the regression) and passes
with the finalizer keyed on the transport.

Verified locally: 1 passed in 11.58s.

---------

Co-authored-by: Sean Ghaeli <ghaeli@amazon.com>
@vatsrahul1001 vatsrahul1001 added this to the Airflow 3.3.0 milestone Jun 23, 2026
@vatsrahul1001 vatsrahul1001 added the type:bug-fix Changelog: Bug Fixes label Jun 23, 2026
cetingokhan pushed a commit to cetingokhan/airflow that referenced this pull request Jun 24, 2026
Through `a2wsgi.ASGIMiddleware`, the `InProcessExecutionAPI` was leaking
a daemon thread per instance, used by `a2wsgi` to run an asyncio event
loop.

This commit moves management of that background thread into
`InProcessExecutionAPI`, and adds a finalizer so that the thread is
stopped when the execution API instance is garbage collected.
cetingokhan pushed a commit to cetingokhan/airflow that referenced this pull request Jun 24, 2026
apache#68865)

* Fix in-process Execution API loop stopped while transport still in use

apache#68840 moved the InProcessExecutionAPI background event-loop + thread
cleanup into a weakref.finalize keyed on the InProcessExecutionAPI
instance. But callers build a sync Client from
InProcessExecutionAPI().transport and discard the factory object, so the
instance is garbage-collected while the transport is still in use. The
finalizer then stops the loop, and every subsequent request hangs on the
dead loop -- surfacing as Timeout failures across the Dag-processor and
triggerer in-process API tests on main (Error while closing in-process
execution API lifespan -> TimeoutError).

Key the finalizer on the returned WSGI transport instead of the factory
instance, so loop/thread/lifespan teardown happens when the transport
(which the Client holds) is collected, not when the throwaway factory is.

Verified locally: test_processor.py::test_top_level_variable_set and
test_top_level_variable_access_not_found now pass (previously hung to
timeout).

* Test transport-tied lifecycle for in-process Execution API

Rewrite test_in_process_execution_api_teardown as
test_in_process_execution_api_transport_lifecycle: assert that dropping
the factory instance leaves the loop running while the transport is held,
and only dropping the transport stops the loop + joins the daemon thread.
This fails against finalizing on the instance (the regression) and passes
with the finalizer keyed on the transport.

Verified locally: 1 passed in 11.58s.

---------

Co-authored-by: Sean Ghaeli <ghaeli@amazon.com>
ashb pushed a commit that referenced this pull request Jun 25, 2026
PR #68840 fixed an issue where exceptions thrown by the FastAPI
lifecycle hook were swallowed. In doing so, it exposed a pre-existing
problem where the lifecycle hook couldn't run when the JWT secret was
not provided.

As the `InProcessExecutionAPI` overrides auth, it doesn't need a JWT
secret, and we certainly don't want to start crashing processes that
previously ran fine as a result of a missing secret that we don't need.

This commit stubs out the registered `JWTValidator` in the FastAPI app
created for the `InProcessExecutionAPI`. This is done in an isolated
services registry to ensure we don't leak this into any real app
instantiations.
ashb pushed a commit that referenced this pull request Jun 25, 2026
…onAPI` (#68980) (#68982)

PR #68840 fixed an issue where exceptions thrown by the FastAPI
lifecycle hook were swallowed. In doing so, it exposed a pre-existing
problem where the lifecycle hook couldn't run when the JWT secret was
not provided.

As the `InProcessExecutionAPI` overrides auth, it doesn't need a JWT
secret, and we certainly don't want to start crashing processes that
previously ran fine as a result of a missing secret that we don't need.

This commit stubs out the registered `JWTValidator` in the FastAPI app
created for the `InProcessExecutionAPI`. This is done in an isolated
services registry to ensure we don't leak this into any real app
instantiations.
(cherry picked from commit e886dfd)

Co-authored-by: Nick Stenning <nick@whiteink.com>
karenbraganz pushed a commit to karenbraganz/airflow that referenced this pull request Jun 30, 2026
…he#68980)

PR apache#68840 fixed an issue where exceptions thrown by the FastAPI
lifecycle hook were swallowed. In doing so, it exposed a pre-existing
problem where the lifecycle hook couldn't run when the JWT secret was
not provided.

As the `InProcessExecutionAPI` overrides auth, it doesn't need a JWT
secret, and we certainly don't want to start crashing processes that
previously ran fine as a result of a missing secret that we don't need.

This commit stubs out the registered `JWTValidator` in the FastAPI app
created for the `InProcessExecutionAPI`. This is done in an isolated
services registry to ensure we don't leak this into any real app
instantiations.
millin added a commit to millin/airflow-dbt-python that referenced this pull request Sep 18, 2026
Airflow <3.3 (fixed upstream by apache/airflow#68840) schedules the
FastAPI execution API app's lifespan startup via
asyncio.run_coroutine_threadsafe without waiting for it, so a
dag.test() task run can race ahead of app.state.svcs_registry being
set and fail with AttributeError. We patch in_process_api_server() to
hand out a pre-warmed transport that waits for lifespan startup,
mirroring the upstream fix, since InProcessExecutionAPI is an
attrs-slotted class we can't otherwise safely monkeypatch.
tomasfarias pushed a commit to tomasfarias/airflow-dbt-python that referenced this pull request Sep 28, 2026
Airflow <3.3 (fixed upstream by apache/airflow#68840) schedules the
FastAPI execution API app's lifespan startup via
asyncio.run_coroutine_threadsafe without waiting for it, so a
dag.test() task run can race ahead of app.state.svcs_registry being
set and fail with AttributeError. We patch in_process_api_server() to
hand out a pre-warmed transport that waits for lifespan startup,
mirroring the upstream fix, since InProcessExecutionAPI is an
attrs-slotted class we can't otherwise safely monkeypatch.
tomasfarias pushed a commit to tomasfarias/airflow-dbt-python that referenced this pull request Sep 28, 2026
Airflow <3.3 (fixed upstream by apache/airflow#68840) schedules the
FastAPI execution API app's lifespan startup via
asyncio.run_coroutine_threadsafe without waiting for it, so a
dag.test() task run can race ahead of app.state.svcs_registry being
set and fail with AttributeError. We patch in_process_api_server() to
hand out a pre-warmed transport that waits for lifespan startup,
mirroring the upstream fix, since InProcessExecutionAPI is an
attrs-slotted class we can't otherwise safely monkeypatch.
tomasfarias pushed a commit to tomasfarias/airflow-dbt-python that referenced this pull request Sep 28, 2026
Airflow <3.3 (fixed upstream by apache/airflow#68840) schedules the
FastAPI execution API app's lifespan startup via
asyncio.run_coroutine_threadsafe without waiting for it, so a
dag.test() task run can race ahead of app.state.svcs_registry being
set and fail with AttributeError. We patch in_process_api_server() to
hand out a pre-warmed transport that waits for lifespan startup,
mirroring the upstream fix, since InProcessExecutionAPI is an
attrs-slotted class we can't otherwise safely monkeypatch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:task-sdk type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants