From 69d68296d89151620ef1270f155001702cf435e0 Mon Sep 17 00:00:00 2001
From: Xavier-Do <xdo@odoo.com>
Date: Wed, 17 May 2023 09:24:48 +0000
Subject: [PATCH] [IMP] core: introduce OrderedWeakSet

The Weakset Used for envs can lead to unpredictible behaviour when
calling flush on the transaction because we iterate on the `envs`.

Since WeakSet.data is a set, the iteration will depends on the python
hash seed.

This commit proposes to replace Transaction.envs.data by an OrderedSet
to solve this issue.

Some test demonstrates that it does not affect the garbage collection
and that the order is now deterministic.

The question to now if we should reverse the envs to find the most
suitable envs remains.

closes odoo/odoo#121604

Signed-off-by: Raphael Collet <rco@odoo.com>
---
 odoo/addons/base/tests/__init__.py          |  1 +
 odoo/addons/base/tests/test_transactions.py | 41 +++++++++++++++++++++
 odoo/api.py                                 |  1 +
 3 files changed, 43 insertions(+)
 create mode 100644 odoo/addons/base/tests/test_transactions.py

diff --git a/odoo/addons/base/tests/__init__.py b/odoo/addons/base/tests/__init__.py
index 49b05903c9b..ee39940dedf 100644
--- a/odoo/addons/base/tests/__init__.py
+++ b/odoo/addons/base/tests/__init__.py
@@ -54,6 +54,7 @@ from . import test_reports
 from . import test_test_retry
 from . import test_test_suite
 from . import test_tests_tags
+from . import test_transactions
 from . import test_form_create
 from . import test_cloc
 from . import test_profiler
diff --git a/odoo/addons/base/tests/test_transactions.py b/odoo/addons/base/tests/test_transactions.py
new file mode 100644
index 00000000000..669f384a44e
--- /dev/null
+++ b/odoo/addons/base/tests/test_transactions.py
@@ -0,0 +1,41 @@
+from odoo.tests.common import TransactionCase
+
+
+class TestTransactionEnvs(TransactionCase):
+    def test_transation_envs_weakrefs(self):
+        transaction = self.env.transaction
+        starting_envs = set(transaction.envs)
+        base_x = self.env['base'].with_context(test_stuff=False)
+        self.assertIn(base_x.env, transaction.envs)
+        del base_x
+        self.assertEqual(set(transaction.envs), starting_envs)
+
+    def do_stuff_with_env(self):
+        base_test = self.env['base'].with_context(test_stuff=False)
+        base_test |= self.env['base'].with_context(test_stuff=1)
+        base_test |= self.env['base'].with_context(test_stuff=2)
+        return base_test
+
+    def test_transation_envs_weakrefs_call(self):
+        transaction = self.env.transaction
+        starting_envs = set(transaction.envs)
+        self.do_stuff_with_env()
+        self.assertEqual(set(transaction.envs), starting_envs)
+
+    def test_transation_envs_weakrefs_return(self):
+        transaction = self.env.transaction
+        starting_envs = set(transaction.envs)
+        base_test = self.do_stuff_with_env()
+        self.assertEqual(set(transaction.envs), starting_envs | {base_test.env})
+
+    def test_transation_envs_ordered(self):
+        transaction = self.env.transaction
+        starting_envs = set(transaction.envs)
+        # create environments in a certain order, not sorted on item
+        items = [3, 8, 1, 5, 2, 7, 6, 9, 0, 4]
+        envs = [self.env(context={'item': item}) for item in items]
+        # check that those environments appear in order in transaction.envs
+        env_items = [env.context['item'] for env in transaction.envs if env not in starting_envs]
+        self.assertEqual(env_items, items)
+        del envs
+        self.assertEqual(set(transaction.envs), starting_envs)
diff --git a/odoo/api.py b/odoo/api.py
index 56c48c414de..9fc9a7618d6 100644
--- a/odoo/api.py
+++ b/odoo/api.py
@@ -825,6 +825,7 @@ class Transaction:
         self.registry = registry
         # weak set of environments
         self.envs = WeakSet()
+        self.envs.data = OrderedSet()  # make the weakset OrderedWeakSet
         # cache for all records
         self.cache = Cache()
         # fields to protect {field: ids}