From 3b861dfe7fe66d5bda5246e6b8f2a8e70e78470b Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 3 Jun 2021 09:26:00 +0000 Subject: [PATCH] [IMP] http: better request context manager The usage of an additional optional context manager leads to the usage of ExitStack. Unfortunately, this changes the initial format quite a lot, decreasing readability and adding a loop on context managers, even if there should be only one most of the time (request). This commit proposes to a nesting utility for the profiler, allowing to nest another context manager inside a profiler. This solution allows an easier integration into http.py, avoids to manage the "enter_context" case when recovering the init stack and can be useful in other cases. --- odoo/http.py | 21 ++++++++++----------- odoo/tools/profiler.py | 28 ++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/odoo/http.py b/odoo/http.py index 8b80f6bebc9..b2feb4f909b 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -1452,10 +1452,11 @@ class Root(object): return request._handle_exception(e) return result - with contextlib.ExitStack() as stack: - for manager in self.get_dispatch_context_managers(request): - stack.enter_context(manager) + request_manager = request + if request.session.profile_session: + request_manager = self.get_profiler_context_manager(request) + with request_manager: db = request.session.db if db: try: @@ -1487,11 +1488,8 @@ class Root(object): except werkzeug.exceptions.HTTPException as e: return e(environ, start_response) - def get_dispatch_context_managers(self, request): - """ Return the context managers to use for dispatching a request. - This includes ``request`` itself, and optionally a profiler. - """ - context_managers = [request] + def get_profiler_context_manager(self, request): + """ Return a context manager that combines a profiler and ``request``. """ if request.session.profile_session and request.session.db: if request.session.profile_expiration < str(datetime.now()): # avoid having session profiling for too long if user forgets to disable profiling @@ -1506,17 +1504,18 @@ class Root(object): _logger.debug("Profiling disabled for evented server") else: try: - context_managers.append(profiler.Profiler( + prof = profiler.Profiler( db=request.session.db, description=request.httprequest.full_path, profile_session=request.session.profile_session, collectors=request.session.profile_collectors, params=request.session.profile_params, - )) + ) + return profiler.Nested(prof, request) except Exception: _logger.exception("Failure during Profiler creation") request.session.profile_session = None - return context_managers + return request def get_db_router(self, db): if not db: diff --git a/odoo/tools/profiler.py b/odoo/tools/profiler.py index f7b3d571295..57353f964e8 100644 --- a/odoo/tools/profiler.py +++ b/odoo/tools/profiler.py @@ -319,8 +319,6 @@ class Profiler: def __enter__(self): self.init_thread = threading.current_thread() self.init_frame = get_current_frame(self.init_thread) - if self.init_frame.f_code.co_name == 'enter_context': - self.init_frame = self.init_frame.f_back # profiler used in a ExitStack case self.init_stack_trace = _get_stack_trace(self.init_frame) if self.description is None: frame = self.init_frame @@ -403,3 +401,29 @@ class Profiler: if filelines is not None: line = filelines[lineno - 1] stack[index] = (filename, lineno, name, line) + + +class Nested: + """ + Utility to nest another context manager inside a profiler. + + The profiler should only be called directly in the "with" without nesting it + with ExitStack. If not, the retrieval of the 'init_frame' may be incorrect + and lead to an error "Limit frame was not found" when profiling. Since the + stack will ignore all stack frames inside this file, the nested frames will + be ignored, too. This is also why Nested() does not use + contextlib.contextmanager. + """ + def __init__(self, profiler, context_manager): + self.profiler = profiler + self.context_manager = context_manager + + def __enter__(self): + self.profiler.__enter__() + return self.context_manager.__enter__() + + def __exit__(self, exc_type, exc_value, traceback): + try: + return self.context_manager.__exit__(exc_type, exc_value, traceback) + finally: + self.profiler.__exit__(exc_type, exc_value, traceback)