From cfadd2bbe45b2de8d060a0ed003ba96e26235722 Mon Sep 17 00:00:00 2001
From: Zhao Chao <zhaochao1984@gmail.com>
Date: Mon, 16 Apr 2018 15:10:25 +0800
Subject: [PATCH] Fix dict iteration in PropertiesCodec

We're updating a dict while iterating in PropertiesCodec, this leads to
random failures of the py35 unittest gate job.

As the logs of a zuul gate job won't be permanent, I wrote a simple
script(though may be ugly) to testing this:

$ python --version
Python 3.5.2

$ cat /tmp/python35_dict_updating.py
data_dict = {}

def init_dict():
    global data_dict
    for x in range(11):
        key = 'key' + str(x+1)
        data_dict[key] = x * 10

def update_dict():
    global data_dict
    for k, v in data_dict.items():
        data_dict.update({k: v+1})

def checking_failed(round):
    global data_dict
    FAILED = False
    for x in range(11):
        key = 'key' + str(x+1)
        origin = x * 10
        current = data_dict[key]
        if (current - origin) != round:
            print('%s is %s, expecting %s' %
                  (key, current, origin + round))
            FAILED = True
            break
    return FAILED

TEST_TIMES=5

PASSED = True
for x in range(TEST_TIMES):
    init_dict()

    for round in range(1, 100):
        update_dict()
        if checking_failed(round):
            print("Failed at round %s" % round)
            PASSED = False
            break

if PASSED:
    print("No failures in 5 100-round tests")

$ python /tmp/python35_dict_updating.py
key9 is 82, expecting 81
Failed at round 1
$ python /tmp/python35_dict_updating.py
key2 is 12, expecting 11
Failed at round 1
$ python /tmp/python35_dict_updating.py
key9 is 82, expecting 81
Failed at round 1
$ python /tmp/python35_dict_updating.py
No failures in 5 100-round tests

From the above testing results, we could see it's quite often that one
of the item will be updated twice during the dict iteration. This is
the reason why test_properties_file_codec failed sometimes.

This fix is manually converting the result of dict.items to a list:

$ sed -i 's/data_dict.items()/list(data_dict.items())/' \
        /tmp/python35_dict_updating.py

$ python /tmp/python35_dict_updating.py
No failures in 5 100-round tests
$ python /tmp/python35_dict_updating.py
No failures in 5 100-round tests
$ python /tmp/python35_dict_updating.py
No failures in 5 100-round tests
$ python /tmp/python35_dict_updating.py
No failures in 5 100-round tests

Closes-Bug: #1764321
Change-Id: Ia9fcfc6519b29f1a9508b79614c5e81456ad57b6
Signed-off-by: Zhao Chao <zhaochao1984@gmail.com>
---
 trove/common/stream_codecs.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/trove/common/stream_codecs.py b/trove/common/stream_codecs.py
index 752dbb9ae6..a85d6c68c1 100644
--- a/trove/common/stream_codecs.py
+++ b/trove/common/stream_codecs.py
@@ -342,7 +342,14 @@ class PropertiesCodec(StreamCodec):
 
         if self._unpack_singletons:
             # Unpack singleton values.
-            for k, v in data_dict.items():
+            # NOTE(zhaochao): In Python 3.x, dict.items() returns a view
+            # object, which will reflect the changes of the dict itself:
+            # https://docs.python.org/3/library/stdtypes.html#dict-views
+            # This means as we're changing the dict, dict.items() cannot
+            # guarantee we're safely iterating all entries in the dict.
+            # Manually converting the result of dict.items() to a list will
+            # fix.
+            for k, v in list(data_dict.items()):
                 data_dict.update({k: trove_utils.unpack_singleton(v)})
 
         return data_dict