[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Osvaldo Santana Neto added the comment: Hi Serhiy Storchaka, I need to thank you for your valuable guidance. -- ___ Python tracker ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: Thank you for your contribution Osvaldo. -- resolution: -> fixed stage: backport needed -> resolved status: open -> closed ___ Python tracker

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: New changeset 1a3bc5546aa27f01426ad76618a9b2c3b698ae68 by Serhiy Storchaka in branch '3.5': [3.5] bpo-30441: Fix bug when modifying os.environ while iterating over it (GH-2409). (#2557)

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: New changeset bebd2cfa5f21811dd0ee4f3b1a1b85d379b83436 by Serhiy Storchaka in branch '3.6': [3.6] bpo-30441: Fix bug when modifying os.environ while iterating over it (GH-2409). (#2556)

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Changes by Serhiy Storchaka : -- pull_requests: +2626 ___ Python tracker ___ ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Changes by Serhiy Storchaka : -- pull_requests: +2625 ___ Python tracker ___ ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Changes by Serhiy Storchaka : -- stage: -> backport needed ___ Python tracker ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: New changeset 8a8d28501fc8ce25926d168f1c657656c809fd4c by Serhiy Storchaka (Osvaldo Santana Neto) in branch 'master': bpo-30441: Fix bug when modifying os.environ while iterating over it (#2409)

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Osvaldo Santana Neto added the comment: Pull Request #2409 (https://github.com/python/cpython/pull/2409) opened. -- pull_requests: +2457 ___ Python tracker

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: Osvaldo, could you please create a pull request on GitHub based on your first patch? But use list() instead of dict(). -- ___ Python tracker

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: See also issue24484. The solution depends on the assumption that list(dict) is atomic. -- ___ Python tracker ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Osvaldo Santana Neto added the comment: New version of patch: 1. Use RLock instead of Lock (Mark & Antoine recomendation) 2. Add lock acquire at `__setitem__` and `__delitem__` as following (Mark & Antoine recomendation) 3. Add lock protection in `__repr__` implementation. I've some questions

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Antoine Pitrou added the comment: Perhaps. But this discussion shows the matter is complicated and we shouldn't rely on fragile assumptions about implementation details. So we need a lock. -- ___ Python tracker

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: Invoking GC before iterating or after iterating doesn't break the atomicity of iterating. -- ___ Python tracker ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Antoine Pitrou added the comment: Every time you create a GC-tracked object, you can invoke the GC. See _PyObject_GC_Alloc. -- ___ Python tracker ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: But it is destroyed only after iterating. -- ___ Python tracker ___ ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Antoine Pitrou added the comment: > While there are no special code for list(dict), regular iteration over exact > dict don't hit the GC. Doesn't it? Creating a dict iterator creates a GC-tracked object. -- ___ Python tracker

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: > There's no special code for list(dict), it uses regular iteration which could > probably hit the GC and then release the GIL. While there are no special code for list(dict), regular iteration over exact dict don't hit the GC. --

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Antoine Pitrou added the comment: > Isn't list(dict) atomic? At least in CPython. I doubt it. There's no special code for list(dict), it uses regular iteration which could probably hit the GC and then release the GIL. > I suspect that protecting *all* accesses to _data with a lock will hit

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Serhiy Storchaka added the comment: Isn't list(dict) atomic? At least in CPython. I suspect that protecting *all* accesses to _data with a lock will hit the performance and invalidate the reason of using _data for caching. -- nosy: +serhiy.storchaka

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Antoine Pitrou added the comment: I'm fine with a low-level lock, though as Mark says you could use _thread._RLock(). -- ___ Python tracker ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Mark Dickinson added the comment: @osantana: you'd need to be using the lock at least in `__setitem__` and `__delitem__` as well, else there's nothing preventing modifications to the dictionary during the `list` call. It may make sense to protect *all* accesses (read or write) to `_data`,

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Osvaldo Santana Neto added the comment: This is an alternative implementation using `threading.Lock()`. The main issue with this implementation relies on the fact that we've to import the `threading` module during CPython interpreter loading (currently it's not imported by default). This is

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Osvaldo Santana Neto added the comment: This patch implements a lock protection (as suggested by Antoine) using `_thread.allocate_lock()` module (instead of `threading.Lock()`) due to the fact that CPython interpreter already imports `_thread` module during its bootstrap process. --

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Antoine Pitrou added the comment: The environ class could have its own lock and then the __iter__ method could be rewritten as follows: def __iter__(self): with self._lock: keys = list(self._data) for key in keys: yield self.decodekey(key)

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Osvaldo Santana Neto added the comment: There are exceptions being raised in many applications (as reported here and in http://bugs.python.org/issue25641) and there are three paths to follow: 1. We handle this exception somewhere; 2. We avoid raising it; 3. Just leave it. I don't care about

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Martin Panter added the comment: Previous report: Issue 25641. At least in Posix, the “putenv” function is not required to be thread safe. -- nosy: +martin.panter ___ Python tracker

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Changes by Danilo Shiga : -- nosy: +Danilo Shiga ___ Python tracker ___ ___

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Osvaldo Santana Neto added the comment: I agree with Jelle about the fix's implementation. But I disagree of suggestion to implement lock at the application side. I disagree because some external libraries may want to access os.environ in a concurrent fashion without lock acquiring. That's

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

Jelle Zijlstra added the comment: Even with the patch, I don't think it's safe to modify os.environ while it's being accessed concurrently in another thread. The other thread's modification could arrive while the dict() call in your patch is running (in CPython the GIL might protect you, but

[issue30441] os.environ raises RuntimeError: dictionary changed size during iteration

New submission from Osvaldo Santana Neto: We're facing ocasional RuntimeError exceptions in a multithreaded application when one of the threads creates new entries to the environment (os.environ). I'm not so sure if the attached patch fixes this issue the right way, so, feel free to propose