From: Christian Heller Date: Sat, 22 Jun 2024 02:43:08 +0000 (+0200) Subject: Empty cache more often to avoid race conditions. X-Git-Url: https://plomlompom.com/repos/%7B%7Bdb.prefix%7D%7D/%7B%7B%20web_path%20%7D%7D/decks/pick_tasks?a=commitdiff_plain;h=7d5347d7bbbbfaf0cbec191ef2dc1ff9cda41ac3;p=plomtask Empty cache more often to avoid race conditions. --- diff --git a/plomtask/http.py b/plomtask/http.py index 417c5a6..0626b4c 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -177,6 +177,38 @@ class TaskHandler(BaseHTTPRequestHandler): @staticmethod def _request_wrapper(http_method: str, not_found_msg: str ) -> Callable[..., Callable[[TaskHandler], None]]: + """Wrapper for do_GET… and do_POST… handlers, to init and clean up. + + Among other things, conditionally cleans all caches, but only on POST + requests, as only those are expected to change the states of objects + that may be cached, and certainly only those are expected to write any + changes to the database. We want to call them as early though as + possible here, either exactly after the specific request handler + returns successfully, or right after any exception is triggered – + otherwise, race conditions become plausible. + + Note that any POST attempt, even a failed one, may end in problematic + inconsistencies: + + - if the POST handler experiences an Exception, changes to objects + won't get written to the DB, but the changed objects may remain in + the cache and affect other objects despite their possibly illegal + state + + - even if an object was just saved to the DB, we cannot be sure its + current state is completely identical to what we'd get if loading it + fresh from the DB (e.g. currently Process.n_owners is only updated + when loaded anew via .from_table_row, nor is its state written to + the DB by .save; a questionable design choice, but proof that we + have no guarantee that objects' .save stores all their states we'd + prefer at their most up-to-date. + """ + + def clear_caches() -> None: + for cls in (Day, Todo, Condition, Process, ProcessStep): + assert hasattr(cls, 'empty_cache') + cls.empty_cache() + def decorator(f: Callable[..., str | None] ) -> Callable[[TaskHandler], None]: def wrapper(self: TaskHandler) -> None: @@ -193,6 +225,8 @@ class TaskHandler(BaseHTTPRequestHandler): if hasattr(self, handler_name): handler = getattr(self, handler_name) redir_target = f(self, handler) + if 'POST' != http_method: + clear_caches() if redir_target: self.send_response(302) self.send_header('Location', redir_target) @@ -201,9 +235,8 @@ class TaskHandler(BaseHTTPRequestHandler): msg = f'{not_found_msg}: {self._site}' raise NotFoundException(msg) except HandledException as error: - for cls in (Day, Todo, Condition, Process, ProcessStep): - assert hasattr(cls, 'empty_cache') - cls.empty_cache() + if 'POST' != http_method: + clear_caches() ctx = {'msg': error} self._send_page(ctx, 'msg', error.http_code) finally: