classification
Title: "RuntimeError: Dictionary changed size during iteration" when copying a WeakValueDictionary
Type: crash Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eamanu, fdrake, ltfish, pitrou, remi.lapeyre, serhiy.storchaka
Priority: normal Keywords: patch, patch, patch, patch

Created on 2018-12-29 20:45 by ltfish, last changed 2019-02-07 20:09 by pitrou. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11384 merged python-dev, 2018-12-31 02:37
PR 11384 merged python-dev, 2018-12-31 02:37
PR 11384 merged python-dev, 2018-12-31 02:37
PR 11384 merged python-dev, 2018-12-31 02:37
PR 11785 merged miss-islington, 2019-02-07 19:52
PR 11785 merged miss-islington, 2019-02-07 19:52
PR 11785 merged miss-islington, 2019-02-07 19:52
Messages (8)
msg332734 - (view) Author: Fish Wang (ltfish) * Date: 2018-12-29 20:45
I come across this issue recently when developing a multi-threaded PySide2 (Qt) application. When I'm calling .copy() on a WeakValueDictionary, there is a high chance that my application crashes with the following stack backtrace:

------

Traceback (most recent call last):
  File "F:\angr\angr-management\angrmanagement\ui\widgets\qdisasm_graph.py", line 239, in mouseDoubleClickEvent
    block.on_mouse_doubleclicked(event.button(), self._to_graph_pos(event.pos()))
  File "F:\angr\angr-management\angrmanagement\ui\widgets\qblock.py", line 130, in on_mouse_doubleclicked
    obj.on_mouse_doubleclicked(button, pos)
  File "F:\angr\angr-management\angrmanagement\ui\widgets\qinstruction.py", line 128, in on_mouse_doubleclicked
    op.on_mouse_doubleclicked(button, pos)
  File "F:\angr\angr-management\angrmanagement\ui\widgets\qoperand.py", line 162, in on_mouse_doubleclicked
    self.disasm_view.jump_to(self._branch_target, src_ins_addr=self.insn.addr)
  File "F:\angr\angr-management\angrmanagement\ui\views\disassembly_view.py", line 258, in jump_to
    self._jump_to(addr)
  File "F:\angr\angr-management\angrmanagement\ui\views\disassembly_view.py", line 372, in _jump_to
    self._display_function(function)
  File "F:\angr\angr-management\angrmanagement\ui\views\disassembly_view.py", line 343, in _display_function
    vr = self.workspace.instance.project.analyses.VariableRecoveryFast(the_func)
  File "f:\angr\angr\angr\analyses\analysis.py", line 109, in __call__
    oself.__init__(*args, **kwargs)
  File "f:\angr\angr\angr\analyses\variable_recovery\variable_recovery_fast.py", line 618, in __init__
    self._analyze()
  File "f:\angr\angr\angr\analyses\forward_analysis.py", line 557, in _analyze
    self._analysis_core_graph()
  File "f:\angr\angr\angr\analyses\forward_analysis.py", line 580, in _analysis_core_graph
    changed, output_state = self._run_on_node(n, job_state)
  File "f:\angr\angr\angr\analyses\variable_recovery\variable_recovery_fast.py", line 712, in _run_on_node
    input_state = prev_state.merge(input_state, successor=node.addr)
  File "f:\angr\angr\angr\analyses\variable_recovery\variable_recovery_fast.py", line 488, in merge
    merged_register_region = self.register_region.copy().replace(replacements).merge(other.register_region,
  File "f:\angr\angr\angr\keyed_region.py", line 159, in copy
    kr._object_mapping = self._object_mapping.copy()
  File "D:\My Program Files\Python37\lib\weakref.py", line 174, in copy
    for key, wr in self.data.items():
RuntimeError: dictionary changed size during iteration

------

I went ahead and read the related methods in Lib\weakref.py, and it seems to me that the WeakValueDictionary.copy() method is missing the protection of an _IterationGuard: It is iterating through self.data.items(), which, might have entries removed because of GC during the iteration.

It seems that this crash can be fixed by wrapping the iteration with `with _IterationGuard(self):`. It worked for me in my tests.

If my above analysis is correct, the following methods all require protection of _IterationGuard (which are currently missing):

- WeakValueDictionary.copy()
- WeakValueDictionary.__deepcopy__()
- WeakKeyDictionary.copy()
- WeakKeyDictionary.__deepcopy__()

Please let me know if this is a legitimate issue, in which case I will be happy to provide a patch. Thanks.
msg332781 - (view) Author: Rémi Lapeyre (remi.lapeyre) * Date: 2018-12-31 01:17
Hi, I think this is indeed a bug, `copy()` is expected to succeed. I was able to reproduce the issue in an interpreter but did not succeed to write a test case that triggers the race-condition.

I think the fix you suggested is the right solution.
msg332789 - (view) Author: Emmanuel Arias (eamanu) * Date: 2018-12-31 02:16
Hi!

Seems like a bug. Please provide a patch. 

This can be apply to 3.8?
msg332790 - (view) Author: Fish Wang (ltfish) * Date: 2018-12-31 02:19
Thanks for your reply.

I'm preparing a PR. However, I'm not sure how to write a reliable test case to trigger the crash outside my application. I will submit the PR for now, and see if anyone on the mailing list has a better idea of what a reliable test case should look like.
msg332793 - (view) Author: Fish Wang (ltfish) * Date: 2018-12-31 02:47
Just submitted a PR against the master branch on GitHub.

> This can be apply to 3.8?

I think so.
msg332794 - (view) Author: Fish Wang (ltfish) * Date: 2018-12-31 02:57
Just checked weakref.py on different branches, I think this bug exists in Python 2.7, 3.4, 3.5, and 3.6 as well.
msg335038 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2019-02-07 19:52
New changeset 96d37dbcd23e65a7a57819aeced9034296ef747e by Antoine Pitrou (Fish) in branch 'master':
bpo-35615: Fix crashes when copying a Weak{Key,Value}Dictionary. (GH-11384)
https://github.com/python/cpython/commit/96d37dbcd23e65a7a57819aeced9034296ef747e
msg335039 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2019-02-07 20:09
New changeset 48769a28ad6ef4183508951fa6a378531ace26a4 by Antoine Pitrou (Miss Islington (bot)) in branch '3.7':
bpo-35615: Fix crashes when copying a Weak{Key,Value}Dictionary. (GH-11384) (GH-11785)
https://github.com/python/cpython/commit/48769a28ad6ef4183508951fa6a378531ace26a4
History
Date User Action Args
2019-02-07 20:09:29pitrousetkeywords: patch, patch, patch, patch
status: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-02-07 20:09:19pitrousetmessages: + msg335039
2019-02-07 19:52:51miss-islingtonsetpull_requests: + pull_request11775
2019-02-07 19:52:43miss-islingtonsetpull_requests: + pull_request11774
2019-02-07 19:52:36miss-islingtonsetpull_requests: + pull_request11773
2019-02-07 19:52:03pitrousetmessages: + msg335038
2018-12-31 06:05:54serhiy.storchakasetkeywords: patch, patch, patch, patch
nosy: + fdrake, pitrou, serhiy.storchaka
2018-12-31 02:57:40ltfishsetmessages: + msg332794
2018-12-31 02:49:58eamanusetversions: + Python 3.8
2018-12-31 02:47:24ltfishsetmessages: + msg332793
2018-12-31 02:37:45python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request10740
2018-12-31 02:37:38python-devsetkeywords: + patch
stage: (no value)
pull_requests: + pull_request10739
2018-12-31 02:37:30python-devsetkeywords: + patch
stage: (no value)
pull_requests: + pull_request10738
2018-12-31 02:37:22python-devsetkeywords: + patch
stage: (no value)
pull_requests: + pull_request10737
2018-12-31 02:19:45ltfishsetmessages: + msg332790
2018-12-31 02:16:53eamanusetnosy: + eamanu
messages: + msg332789
2018-12-31 01:17:43remi.lapeyresetnosy: + remi.lapeyre
messages: + msg332781
2018-12-29 20:45:37ltfishcreate