bottlepy / bottle

bottle.py is a fast and simple micro-framework for python web-applications.
http://bottlepy.org/
MIT License
8.39k stars 1.46k forks source link

global request/response is not coroutine safe #1330

Closed nu774 closed 3 years ago

nu774 commented 3 years ago

Consider example like the following:

@get('/empty')
def empty():
    response.status = 204

@get('/longpoll')
def longpoll():
    response.content_type = 'application/json'
    time.sleep(10)
    return {'value': time.asctime() }

When bottle is condifigured to use greenlet based WSGI impl (such as gevent or eventlet) and time.sleep() is monkey-patched, longpoll() seemingly works fine, but it actually does not.

If the app receives a request to /empty while waiting in time.sleep() in longpoll(), response is overwritten by the empty(). So, long poll returns 204 No Content instead of the expected JSON response.

In other word, the design of response as global variable is flawed, and needs care when used with greenlet. At least this is documented.

oz123 commented 3 years ago

Yes it's documented. Where is the issue?

nu774 commented 3 years ago

Oh, sorry... Then I have missed that one. But where? I didn't find it in the following: https://bottlepy.org/docs/dev/api.html#module-contents https://bottlepy.org/docs/dev/async.html#greenlets-to-the-rescue https://bottlepy.org/docs/dev/faq.html#common-problems-and-pitfalls

defnull commented 3 years ago

The request/response 'globals' are actually thread local and documented as such in several places. By using a greenlet-functions within a request callback, you break some fundamental assumptions about threads without telling bottle about it. You allow multiple concurrent request to be served in the same thread, breaking the thread locality of the request and response objects.

It is not enough to just monkey-patch methods you use in your application code, you must also patch thread-related logic in libraries you use. This includes threading.local (and be sure to patch that before importing bottle). Both the bottle async primer and the greenlet docs tell you that. The 'gevent' server adapter even goes so far and warns you if you missed that detail.

I agree that the async primer is worded a bit misleadingly and a PR to clear up this misunderstanding would be nice. The example is correct, though.

nu774 commented 3 years ago

OK, thanks. Closing this issue.