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

ClosingIterator does not close wrapped iterable #1259

Closed
romank0 opened this issue Mar 2, 2018 · 0 comments

Comments

Projects
None yet
2 participants
@romank0
Copy link
Contributor

commented Mar 2, 2018

ClosingIterator does not respect wsgi specification namely this piece:

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

The problem is that ClosingIterator tries to invoke close on the iterator returned by the iterable but it should invoke close on the iterable itself.

Here's relevant part of the example server implementation from the wsgi specification:

    result = application(environ, start_response)
    try:
        for data in result:
            if data:    # don't send headers until body appears
                write(data)
        if not headers_sent:
            write('')   # send headers now if body was empty
    finally:
        if hasattr(result, 'close'):
            result.close()

Note that close is invoked on the object returned by the application.

This causes the problem as close is not invoked at all. I face this problem in a django application that runs in gunicorn which is setup to use gevent async workers. In order to isolate greenlets werkzeug LocalManager is used to wrap django wsgi application. As close is not invoked django does not do necessary cleanup and does not close database connections. The problem that is described in django documentation happens:

screen shot 2018-03-01 at 14 44 09

Here's the smallest example that demonstrates the problem.
I used AppClass from the wsgi specification with a slight modification that adds a close method to it.

  1. install two requirements into virtual env:
uwsgi==2.0.17
Werkzeug==0.14.1
  1. create app.py file with content below
  2. run server bin/uwsgi --http :9090 --wsgi-file app.py
  3. execute request curl "http://localhost:9090"

If you set wrap to False wsgi application is not wrapped and closed is printed to stdout. If the application is wrapped with ClosingIterator then closed is not invoked and nothing is printed.

app.py file:

class AppClass:
    def __init__(self, environ, start_response):
        self.start = start_response

    def __iter__(self):
        status = '200 OK'
        response_headers = [('Content-type', 'text/plain')]
        self.start(status, response_headers)
        yield b"Hello World"

    def close(self):
        print "closed"

wrap = True

if wrap:
    def application(environ, start_response):
        from werkzeug.wsgi import ClosingIterator
        return ClosingIterator(AppClass(environ, start_response))
else:
    application = AppClass

romank0 added a commit to romank0/werkzeug that referenced this issue Mar 2, 2018

ClosingIterator closes iterable passed to it.
This is a fix for pallets#1259.

According to [wsgi specification](https://www.python.org/dev/peps/pep-3333):

> 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

This commit adds test for this and fixes the issue.

@davidism davidism added this to the 0.15 milestone May 30, 2018

davidism added a commit to romank0/werkzeug that referenced this issue May 30, 2018

ClosingIterator closes iterable passed to it.
This is a fix for pallets#1259.

According to [wsgi specification](https://www.python.org/dev/peps/pep-3333):

> 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

This commit adds test for this and fixes the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.