From c4ddbb064adb1ede22f880984bfa411196feb37b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Sat, 2 Mar 2019 23:11:14 +0100 Subject: [PATCH] Prevent filling cache with data from old transaction Once the cache has been cleared, we must ensure that transaction started before the clearing cannot fill the global cache. issue8019 review56541002 --- doc/ref/transaction.rst | 8 ++++++++ trytond/cache.py | 13 ++++++++++++- trytond/tests/test_cache.py | 21 +++++++++++++++++++++ trytond/transaction.py | 13 +++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/doc/ref/transaction.rst b/doc/ref/transaction.rst index 7657de5ba5..8ae239f718 100644 --- a/doc/ref/transaction.rst +++ b/doc/ref/transaction.rst @@ -37,6 +37,10 @@ commited. .. attribute:: Transaction.timestamp +.. attribute:: Transaction.started_at + + The monotonic timestamp when the transaction started. + .. attribute:: Transaction.language The language code defines in the context. @@ -45,6 +49,10 @@ commited. Count the number of modification made in this transaction. +.. staticmethod:: Transaction.monotonic_time + + Return a monotonic time used to populate :attr:~Transaction.started_at. + .. method:: Transaction.start(database_name, user[, readonly[, context[, close[, autocommit]]]]) Start a new transaction and return a `context manager`_. The non-readonly diff --git a/trytond/cache.py b/trytond/cache.py index fdf2cbfe17..e73ff96d88 100644 --- a/trytond/cache.py +++ b/trytond/cache.py @@ -83,17 +83,21 @@ class MemoryCache(BaseCache): """ _reset = WeakKeyDictionary() _clean_last = datetime.now() + _default_lower = Transaction.monotonic_time() def __init__(self, name, size_limit=1024, context=True): super(MemoryCache, self).__init__(name, size_limit, context) self._database_cache = defaultdict(lambda: LRUDict(size_limit)) self._transaction_cache = WeakKeyDictionary() + self._transaction_lower = {} self._timestamp = {} def _get_cache(self): transaction = Transaction() dbname = transaction.database.name - if transaction in self._reset: + lower = self._transaction_lower.get(dbname, self._default_lower) + if (transaction in self._reset + or transaction.started_at < lower): try: return self._transaction_cache[transaction] except KeyError: @@ -145,6 +149,9 @@ class MemoryCache(BaseCache): if not inst_timestamp or timestamp > inst_timestamp: inst._timestamp[dbname] = timestamp inst._database_cache[dbname] = LRUDict(inst.size_limit) + inst._transaction_lower[dbname] = max( + transaction.monotonic_time(), + inst._transaction_lower.get(dbname, cls._default_lower)) cls._clean_last = datetime.now() @classmethod @@ -177,6 +184,9 @@ class MemoryCache(BaseCache): inst = cls._instances[name] inst._timestamp[dbname] = timestamp inst._database_cache[dbname] = LRUDict(inst.size_limit) + inst._transaction_lower[dbname] = max( + transaction.monotonic_time(), + inst._transaction_lower.get(dbname, cls._default_lower)) @classmethod def rollback(cls, transaction): @@ -190,6 +200,7 @@ class MemoryCache(BaseCache): for inst in cls._instances.values(): inst._timestamp.pop(dbname, None) inst._database_cache.pop(dbname, None) + inst._transaction_lower.pop(dbname, None) if config.get('cache', 'class'): diff --git a/trytond/tests/test_cache.py b/trytond/tests/test_cache.py index 99cacdc782..537c204a42 100644 --- a/trytond/tests/test_cache.py +++ b/trytond/tests/test_cache.py @@ -102,6 +102,27 @@ class CacheTestCase(unittest.TestCase): with Transaction().start(DB_NAME, USER): self.assertGreater(cache._clean_last, last) + def test_memory_cache_old_transaction(self): + "Test old transaction does not fill cache" + transaction1 = Transaction().start(DB_NAME, USER) + self.addCleanup(transaction1.stop) + + # Clear cache from new transaction + transaction2 = transaction1.new_transaction() + self.addCleanup(transaction2.stop) + cache.clear() + transaction2.commit() + + # Set value from old transaction + Transaction().set_current_transaction(transaction1) + self.addCleanup(transaction1.stop) + cache.set('foo', 'baz') + + # New transaction has still empty cache + transaction3 = transaction1.new_transaction() + self.addCleanup(transaction3.stop) + self.assertEqual(cache.get('foo'), None) + def suite(): func = unittest.TestLoader().loadTestsFromTestCase diff --git a/trytond/transaction.py b/trytond/transaction.py index 323e6b65dd..f52d6c2494 100644 --- a/trytond/transaction.py +++ b/trytond/transaction.py @@ -1,6 +1,7 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. import logging +import time from threading import local from sql import Flavor @@ -53,6 +54,7 @@ class Transaction(object): delete_records = None delete = None # TODO check to merge with delete_records timestamp = None + started_at = None def __new__(cls, new=False): transactions = cls._local.transactions @@ -65,6 +67,13 @@ class Transaction(object): instance = transactions[-1] return instance + @staticmethod + def monotonic_time(): + try: + return time.monotonic_ns() + except AttributeError: + return time.monotonic() + @property def tasks(self): return self._local.tasks @@ -87,6 +96,10 @@ class Transaction(object): assert self.database is None assert self.close is None assert self.context is None + # Compute started_at before connect to ensure + # it is strictly before all transactions started after + # but it may be also before transactions started before + self.started_at = self.monotonic_time() if not database_name: database = Database().connect() else: