From 683a51bd277262908cc4f124e3028ff48bf1ab63 Mon Sep 17 00:00:00 2001 From: "Carsten Wolff (cawo)" Date: Thu, 18 Jan 2024 09:17:44 +0000 Subject: [PATCH] [FIX] http: close request resources when done Currently, when a worker (process/thread) has finished processing a request, it will keep handles on resources held by the werkzeug `Request` object. This includes open filhandles to temporary files, e.g. those of uploaded files. On platforms supporting `O_TMPFILE`, these files are not visible in the filesystem, but keep using up space in `TMPDIR` until werkzeug finally closes the file handles when the next Request is being handled. In some contexts, e.g. the upgrade platform, it can happen that there are multiple workers that only handle rare requests that upload big files (multiple GiB), kept open after the upload has finished: ```shell lsof -nP | grep -E 'odoo\/tmp.*(deleted)' | grep -vE 'GeoIP' python3 213853 odoo 13u REG 252,3 1064251 926275 /home/odoo/tmp/#926275 (deleted) python3 213853 213865 python3 odoo 13u REG 252,3 1064251 926275 /home/odoo/tmp/#926275 (deleted) ``` This can pose problems, because often the filesystem on `TMPDIR` is not very large and idle workers holding on to large files can increase the chance for ENOSPC. This patch changes the behavior such that the resources held by the werkzeug `Request` object are being closed[^1] after the response has been sent out. This also has the advantage that this work is done at potentially idle time instead of within handling the next request. [^1]: https://werkzeug.palletsprojects.com/en/2.0.x/wrappers/#werkzeug.wrappers.Request.close closes odoo/odoo#150029 X-original-commit: a920ddca57e7bf9353611396f1c0b1060c8cb044 Signed-off-by: Julien Castiaux (juc) --- odoo/http.py | 82 ++++++++++++++++++++++++++-------------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/odoo/http.py b/odoo/http.py index a2967cd41fe..32ad8b31ade 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -2132,53 +2132,53 @@ class Application: return ProxyFix(fake_app)(environ, fake_start_response) - httprequest = werkzeug.wrappers.Request(environ) - httprequest.user_agent_class = UserAgent # use vendored userAgent since it will be removed in 2.1 - httprequest.parameter_storage_class = ( - werkzeug.datastructures.ImmutableOrderedMultiDict) - httprequest.max_content_length = DEFAULT_MAX_CONTENT_LENGTH - request = Request(httprequest) - _request_stack.push(request) - request._post_init() - current_thread.url = httprequest.url + with werkzeug.wrappers.Request(environ) as httprequest: + httprequest.user_agent_class = UserAgent # use vendored userAgent since it will be removed in 2.1 + httprequest.parameter_storage_class = ( + werkzeug.datastructures.ImmutableOrderedMultiDict) + httprequest.max_content_length = DEFAULT_MAX_CONTENT_LENGTH + request = Request(httprequest) + _request_stack.push(request) + request._post_init() + current_thread.url = httprequest.url - try: - if self.get_static_file(httprequest.path): - response = request._serve_static() - elif request.db: - with request._get_profiler_context_manager(): - response = request._serve_db() - else: - response = request._serve_nodb() - return response(environ, start_response) - - except Exception as exc: - # Valid (2xx/3xx) response returned via werkzeug.exceptions.abort. - if isinstance(exc, HTTPException) and exc.code is None: - response = exc.get_response() - HttpDispatcher(request).post_dispatch(response) + try: + if self.get_static_file(httprequest.path): + response = request._serve_static() + elif request.db: + with request._get_profiler_context_manager(): + response = request._serve_db() + else: + response = request._serve_nodb() return response(environ, start_response) - # Logs the error here so the traceback starts with ``__call__``. - if hasattr(exc, 'loglevel'): - _logger.log(exc.loglevel, exc, exc_info=getattr(exc, 'exc_info', None)) - elif isinstance(exc, HTTPException): - pass - elif isinstance(exc, SessionExpiredException): - _logger.info(exc) - elif isinstance(exc, (UserError, AccessError, NotFound)): - _logger.warning(exc) - else: - _logger.error("Exception during request handling.", exc_info=True) + except Exception as exc: + # Valid (2xx/3xx) response returned via werkzeug.exceptions.abort. + if isinstance(exc, HTTPException) and exc.code is None: + response = exc.get_response() + HttpDispatcher(request).post_dispatch(response) + return response(environ, start_response) - # Ensure there is always a WSGI handler attached to the exception. - if not hasattr(exc, 'error_response'): - exc.error_response = request.dispatcher.handle_error(exc) + # Logs the error here so the traceback starts with ``__call__``. + if hasattr(exc, 'loglevel'): + _logger.log(exc.loglevel, exc, exc_info=getattr(exc, 'exc_info', None)) + elif isinstance(exc, HTTPException): + pass + elif isinstance(exc, SessionExpiredException): + _logger.info(exc) + elif isinstance(exc, (UserError, AccessError, NotFound)): + _logger.warning(exc) + else: + _logger.error("Exception during request handling.", exc_info=True) - return exc.error_response(environ, start_response) + # Ensure there is always a WSGI handler attached to the exception. + if not hasattr(exc, 'error_response'): + exc.error_response = request.dispatcher.handle_error(exc) - finally: - _request_stack.pop() + return exc.error_response(environ, start_response) + + finally: + _request_stack.pop() root = Application()