Skip to content

Fix request resource leak when teardown handlers raise#5911

Closed
THE-Amrit-mahto-05 wants to merge 1 commit intopallets:mainfrom
THE-Amrit-mahto-05:fix-request-close-on-teardown-error
Closed

Fix request resource leak when teardown handlers raise#5911
THE-Amrit-mahto-05 wants to merge 1 commit intopallets:mainfrom
THE-Amrit-mahto-05:fix-request-close-on-teardown-error

Conversation

@THE-Amrit-mahto-05
Copy link

Ensure that request.close() is always executed even if
do_teardown_request() raises an exception.

Currently, if a user-defined teardown handler raises an exception,
request.close() is skipped, which can lead to resource leaks

This change wraps teardown execution in a try/finally block to
guarantee cleanup without altering normal behavior.

@davidism davidism closed this Feb 6, 2026
@davidism
Copy link
Member

davidism commented Feb 6, 2026

From the docs: https://flask.palletsprojects.com/en/stable/reqcontext/#teardown-callbacks

Be sure to write these functions in a way that does not depend on other callbacks and will not fail.

There are a lot more places than this in pop that could result in leftover state, which is why we warn that teardown functions must not fail. For example, if a teardown function fails, any remaining are not run. In that sense, close is just another teardown function. As to why the existing finally is there in that case, no idea.

@THE-Amrit-mahto-05
Copy link
Author

@davidism
I’m not trying to make teardown callbacks safe to fail , only to ensure that framework owned cleanup (request.close()) is not skipped when they do

@davidism
Copy link
Member

davidism commented Feb 6, 2026

What I'm saying is that adding a finally here doesn't really help, as any callback failing means that other resources are also potentially left dangling. In that case, the issue is with the callback, and needs to be fixed there.

I've been working on closing resources over in Werkzeug's tests, maybe that's what prompted you to look at this. I'm also working on that here, just haven't pushed the changes yet. Thanks for pointing out this area, if nothing else I'll move the appcontext teardown out of the finally and into the try so that the context is always cleared. Perhaps there's also a case for try/except pass around every teardown function, to clean up as much as possible, I'll look into that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments