[Fix][Issue 107] RuntimeError: dictionary changed size during iteration

[viniarck]

Fixes #107

• Shallow copy shared iterables that are used on REST endpoints
• Refactored previous shallow copies to use .copy() to make the intention and code clearear

Demonstrating the problem

The underlying issue, as explicit summarized in the exception, is related to a shared memory data struct being changed while it's being iterated (a best practice in kytos-ng docs to avoid this will be addressed on kytos-ng/kytos-ng.github.io#33):

• Here's a minimal example with two temporary functions that iterate on a shared iterable controller.switches (self.switches), and intentionally switches_iter_changing_size will insert into the collection to simulate the issue, whereas switches_iter_copy_changing_size will make a shallow copy and iterate on it, while inserting into the original collection:

diff --git a/kytos/core/controller.py b/kytos/core/controller.py
index ccb6c5a..1b42103 100644
--- a/kytos/core/controller.py
+++ b/kytos/core/controller.py
@@ -200,6 +200,18 @@ class Controller:
                 for name in logging.root.manager.loggerDict
                 if "kytos" in name]

+    def switches_iter_changing_size(self) -> None:
+        for idx, switch in enumerate(self.switches):
+            self.log.info(f"idx {idx}, it'll add switch {idx}")
+            switch = Switch(str(idx), connection=None)
+            self.add_new_switch(switch)
+
+    def switches_iter_copy_changing_size(self) -> None:
+        for idx, switch in enumerate(dict(self.switches)):
+            self.log.info(f"idx {idx}, it'll add switch {idx}")
+            switch = Switch(str(idx), connection=None)
+            self.add_new_switch(switch)
+
     def toggle_debug(self, name=None):
         """Enable/disable logging debug messages to a given logger name.

• As you'd expect switches_iter_changing_size raises RuntimeError whereas switches_iter_changing_size doesnt:

kytos $> controller.switches_iter_copy_changing_size()
2022-08-09 12:39:08,073 - INFO [kytos.core.controller] (ThreadPoolExecutor-1_0) idx 0, it'll add switch 0
2022-08-09 12:39:08,075 - INFO [kytos.core.controller] (ThreadPoolExecutor-1_0) idx 1, it'll add switch 1
2022-08-09 12:39:08,076 - INFO [kytos.core.controller] (ThreadPoolExecutor-1_0) idx 2, it'll add switch 2

kytos $> controller.switches_iter_changing_size()
2022-08-09 12:39:14,582 - INFO [kytos.core.controller] (ThreadPoolExecutor-1_0) idx 0, it'll add switch 0
2022-08-09 12:39:14,584 - INFO [kytos.core.controller] (ThreadPoolExecutor-1_0) idx 1, it'll add switch 1
2022-08-09 12:39:14,586 - INFO [kytos.core.controller] (ThreadPoolExecutor-1_0) idx 2, it'll add switch 2
2022-08-09 12:39:14,587 - INFO [kytos.core.controller] (ThreadPoolExecutor-1_0) idx 3, it'll add switch 3
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
~/repos/kytos/kytos/core/kytosd.py in <module>
----> 1 controller.switches_iter_changing_size()

~/repos/kytos/kytos/core/controller.py in switches_iter_changing_size(self)
    204
    205     def switches_iter_changing_size(self) -> None:
--> 206         for idx, switch in enumerate(self.switches):
    207             self.log.info(f"idx {idx}, it'll add switch {idx}")
    208             switch = Switch(str(idx), connection=None)

RuntimeError: dictionary changed size during iteration

Performance

• It'll depend on object size and the collection size, but since NApps are levering MongoDB as much as possible and avoiding to store large collections in memory, shallow copies isn't expected to introduce much latency, here's a benchmark with a collection with 10000 elements, it added roughly 100 us on mean and median results:

class Helper:
    @classmethod
    def gen_dict(cls, size: int) -> dict:
        return {i: i for i in range(size)}

    @classmethod
    def iter_dict(cls, size: int) -> None:
        """Perform an iteration simulating an conditional too"""
        for k, v in Helper.gen_dict(size).items():
            if k == "x":
                pass

    @classmethod
    def iter_dict_copy(cls, size: int) -> None:
        """Perform an iteration shallow copy simulating an conditional too"""
        for k, v in Helper.gen_dict(size).copy().items():
            if k == "x":
                pass


def test_dict_iter_10000(benchmark) -> None:
    benchmark(Helper.iter_dict, 10000)


def test_dict_iter_10000_copy(benchmark) -> None:
    benchmark(Helper.iter_dict_copy, 10000)

================================================================================== test session starts ===================================================================================
platform linux -- Python 3.10.5, pytest-7.1.2, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/viniarck/repos/python
plugins: benchmark-3.4.1
collected 2 items

test_copy_bench.py ..                                                                                                                                                              [100%]


------------------------------------------------------------------------------------------- benchmark: 2 tests ---------------------------------------------------------------------------
----------------
Name (time in us)                  Min                   Max                Mean             StdDev              Median                IQR            Outliers  OPS (Kops/s)            Ro
unds  Iterations
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
----------------
test_dict_iter_10000          705.5320 (1.0)      1,479.0870 (1.28)     747.5517 (1.0)      51.5913 (1.37)     732.8570 (1.0)      34.6405 (1.58)        82;64        1.3377 (1.0)
1131           1
test_dict_iter_10000_copy     812.9850 (1.15)     1,152.3990 (1.0)      852.5019 (1.14)     37.5999 (1.0)      841.0625 (1.15)     21.9300 (1.0)       133;118        1.1730 (0.88)
1150           1
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
----------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
=================================================================================== 2 passed in 2.84s ====================================================================================

[viniarck]

Thanks for reviewing, Antonio