From 375f977e892b94579d017461578cd6cef6cf5ebc Mon Sep 17 00:00:00 2001 From: Osvaldo Santana Neto Date: Mon, 26 Jun 2017 11:23:27 -0300 Subject: [PATCH 1/7] Fix bug when modifying os.environ while iterating over it (Issue #30441) --- Lib/os.py | 3 ++- Lib/test/test_os.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Lib/os.py b/Lib/os.py index e293ecae7fd3a4..51d3d030281955 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -697,7 +697,8 @@ def __delitem__(self, key): raise KeyError(key) from None def __iter__(self): - for key in self._data: + data = list(self._data) + for key in data: yield self.decodekey(key) def __len__(self): diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index d06927073edf0e..115404f6a68581 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -835,6 +835,21 @@ def test_key_type(self): self.assertIs(cm.exception.args[0], missing) self.assertTrue(cm.exception.__suppress_context__) + def test_iter_error_when_os_environ_changes(self): + def _iter_environ_change(): + for key, value in os.environ.items(): + yield key, value + + iter_environ = _iter_environ_change() + key, value = next(iter_environ) # start iteration over os.environ + + # add a new key in os.environ mapping + new_key = "__{}".format(key) + os.environ[new_key] = value + + next(iter_environ) # force iteration over modified mapping + self.assertEqual(os.environ[new_key], value) + class WalkTests(unittest.TestCase): """Tests for os.walk().""" From cac586f5ddb7aecc106f7b3370295d59decb8857 Mon Sep 17 00:00:00 2001 From: Osvaldo Santana Neto Date: Thu, 29 Jun 2017 14:16:23 -0300 Subject: [PATCH 2/7] Rename data->keys and explain about list() atomicity --- Lib/os.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/os.py b/Lib/os.py index 51d3d030281955..807ddb56c06558 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -697,8 +697,9 @@ def __delitem__(self, key): raise KeyError(key) from None def __iter__(self): - data = list(self._data) - for key in data: + # list() from dict object is an atomic operation + keys = list(self._data) + for key in keys: yield self.decodekey(key) def __len__(self): From 6bbfe8cafdae9f4d48ff1ee8cf95b1b1ad647c08 Mon Sep 17 00:00:00 2001 From: Osvaldo Santana Neto Date: Thu, 29 Jun 2017 14:16:54 -0300 Subject: [PATCH 3/7] Improve tests and add more use-cases --- Lib/test/test_os.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 115404f6a68581..facf3a5d4ea24f 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -835,20 +835,30 @@ def test_key_type(self): self.assertIs(cm.exception.args[0], missing) self.assertTrue(cm.exception.__suppress_context__) - def test_iter_error_when_os_environ_changes(self): - def _iter_environ_change(): - for key, value in os.environ.items(): - yield key, value + def _test_environ_iteration(self, iterator): + new_key = "__new_key__" - iter_environ = _iter_environ_change() - key, value = next(iter_environ) # start iteration over os.environ + next(iterator) # start iteration over os.environ.items # add a new key in os.environ mapping - new_key = "__{}".format(key) - os.environ[new_key] = value + os.environ[new_key] = "test_environ_iteration" - next(iter_environ) # force iteration over modified mapping - self.assertEqual(os.environ[new_key], value) + next(iterator) # force iteration over modified mapping + self.assertEqual(os.environ[new_key], "test_environ_iteration") + + del os.environ[new_key] + + def test_iter_error_when_changin_os_environ(self): + iter_environ = iter(os.environ) + self._test_environ_iteration(iter_environ) + + def test_iter_error_when_changin_os_environ_items(self): + iter_environ = iter(os.environ.items()) + self._test_environ_iteration(iter_environ) + + def test_iter_error_when_changin_os_environ_values(self): + iter_environ = iter(os.environ.values()) + self._test_environ_iteration(iter_environ) class WalkTests(unittest.TestCase): From 21909318aa26962194abeb0de705f06a00b01c89 Mon Sep 17 00:00:00 2001 From: Osvaldo Santana Neto Date: Thu, 29 Jun 2017 14:28:53 -0300 Subject: [PATCH 4/7] Add entry in NEWS.d/Library and author name in Misc/ACKS --- Misc/ACKS | 1 + .../NEWS.d/next/Library/2017-06-29-14-25-14.bpo-30441.3Wh9kc.rst | 1 + 2 files changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2017-06-29-14-25-14.bpo-30441.3Wh9kc.rst diff --git a/Misc/ACKS b/Misc/ACKS index eaff17232c9a12..b5277b0a9de090 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1760,3 +1760,4 @@ Jelle Zijlstra Gennadiy Zlobin Doug Zongker Peter Åstrand +Osvaldo Santana Neto diff --git a/Misc/NEWS.d/next/Library/2017-06-29-14-25-14.bpo-30441.3Wh9kc.rst b/Misc/NEWS.d/next/Library/2017-06-29-14-25-14.bpo-30441.3Wh9kc.rst new file mode 100644 index 00000000000000..55dd6136c8d708 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-06-29-14-25-14.bpo-30441.3Wh9kc.rst @@ -0,0 +1 @@ +Fix bug when modifying os.environ while iterating over it From 3b15ce05285b1f8dee3bbd5fdae468336e617895 Mon Sep 17 00:00:00 2001 From: Osvaldo Santana Neto Date: Thu, 29 Jun 2017 14:31:53 -0300 Subject: [PATCH 5/7] Fix typo in test names and improve var names --- Lib/test/test_os.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index facf3a5d4ea24f..dfef4e3d7f69a9 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -848,17 +848,17 @@ def _test_environ_iteration(self, iterator): del os.environ[new_key] - def test_iter_error_when_changin_os_environ(self): + def test_iter_error_when_changing_os_environ(self): iter_environ = iter(os.environ) self._test_environ_iteration(iter_environ) - def test_iter_error_when_changin_os_environ_items(self): - iter_environ = iter(os.environ.items()) - self._test_environ_iteration(iter_environ) + def test_iter_error_when_changing_os_environ_items(self): + iter_environ_items = iter(os.environ.items()) + self._test_environ_iteration(iter_environ_items) - def test_iter_error_when_changin_os_environ_values(self): - iter_environ = iter(os.environ.values()) - self._test_environ_iteration(iter_environ) + def test_iter_error_when_changing_os_environ_values(self): + iter_environ_values = iter(os.environ.values()) + self._test_environ_iteration(iter_environ_values) class WalkTests(unittest.TestCase): From a061788102ccefd1e02483216a2edfa88bbd3e1f Mon Sep 17 00:00:00 2001 From: Osvaldo Santana Neto Date: Sat, 1 Jul 2017 13:27:55 -0300 Subject: [PATCH 6/7] Refactor tests to improve readability --- Lib/test/test_os.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index dfef4e3d7f69a9..49e5a37cd209d7 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -835,7 +835,8 @@ def test_key_type(self): self.assertIs(cm.exception.args[0], missing) self.assertTrue(cm.exception.__suppress_context__) - def _test_environ_iteration(self, iterator): + def _test_environ_iteration(self, collection): + iterator = iter(collection) new_key = "__new_key__" next(iterator) # start iteration over os.environ.items @@ -843,22 +844,20 @@ def _test_environ_iteration(self, iterator): # add a new key in os.environ mapping os.environ[new_key] = "test_environ_iteration" - next(iterator) # force iteration over modified mapping - self.assertEqual(os.environ[new_key], "test_environ_iteration") - - del os.environ[new_key] + try: + next(iterator) # force iteration over modified mapping + self.assertEqual(os.environ[new_key], "test_environ_iteration") + finally: + del os.environ[new_key] def test_iter_error_when_changing_os_environ(self): - iter_environ = iter(os.environ) - self._test_environ_iteration(iter_environ) + self._test_environ_iteration(os.environ) def test_iter_error_when_changing_os_environ_items(self): - iter_environ_items = iter(os.environ.items()) - self._test_environ_iteration(iter_environ_items) + self._test_environ_iteration(os.environ.items()) def test_iter_error_when_changing_os_environ_values(self): - iter_environ_values = iter(os.environ.values()) - self._test_environ_iteration(iter_environ_values) + self._test_environ_iteration(os.environ.values()) class WalkTests(unittest.TestCase): From 4c3f4dae887dd5e58829151601c6bb0c3e48b6d3 Mon Sep 17 00:00:00 2001 From: Osvaldo Santana Neto Date: Sat, 1 Jul 2017 13:34:30 -0300 Subject: [PATCH 7/7] Fix surname alphabetical order in Misc/ACKS --- Misc/ACKS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/ACKS b/Misc/ACKS index b5277b0a9de090..f1bb3e6a3a2e45 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1087,6 +1087,7 @@ Fredrik Nehr Tony Nelson Trent Nelson Andrew Nester +Osvaldo Santana Neto Chad Netzer Max Neunhöffer Anthon van der Neut @@ -1760,4 +1761,3 @@ Jelle Zijlstra Gennadiy Zlobin Doug Zongker Peter Åstrand -Osvaldo Santana Neto