Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WSGI specification requires that close() be called on iterable no matter what happens. #824

Merged
merged 1 commit into from
Jun 18, 2013
Merged

Conversation

GrahamDumpleton
Copy link
Contributor

Tornado does not comply with the WSGI specification (PEP 333/3333).

Specifically the WSGI specification says:

"""If the iterable returned by the application has a close() method, the server or gateway must call that method upon completion of the current request, whether the request was completed normally, or terminated early due to an application error during iteration or an early disconnect of the browser."""

Tornado is not calling close() when an unhandled exception propagates all the way up to the WSGIContainer.

This can cause problems for WSGI applications or middleware which rely on close() being called at the end of the request to cleanup resources or end transactions.

The requirement is something that uWSGI, wsgiref, Sentry and others have got wrong at times and has been blogged about in:

http://blog.dscpl.com.au/2012/10/obligations-for-calling-close-on.html

in order to highlight the problem, yet Tornado still has got it wrong.

@bdarnell
Copy link
Member

Looks good to me. Are there any good test suites for WSGI compliance besides wsgiref.validate?

bdarnell added a commit that referenced this pull request Jun 18, 2013
WSGI specification requires that close() be called on iterable no matter what happens.
@bdarnell bdarnell merged commit 913997e into tornadoweb:master Jun 18, 2013
@GrahamDumpleton
Copy link
Contributor Author

No, there are no comprehensive test suites that I know of. I have tried to organise my own but just haven't had the time.

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.

None yet

2 participants