[issue19542] WeakValueDictionary bug in setdefault()()

[Donald Stufft]

Changes by Donald Stufft :


--
pull_requests: +828

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Antoine Pitrou]

Antoine Pitrou added the comment:

This is expected, because of https://bugs.python.org/issue28427

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Arfrever Frehtes Taifersar Arahesis]

Arfrever Frehtes Taifersar Arahesis added the comment:

Newly added test_threaded_weak_valued_pop() triggers ignored KeyError 
exceptions (on all branches):

$ python3.7 -m test -v test_weakref
...
0:00:00 [1/1] test_weakref
...
test_make_weak_keyed_dict_from_dict (test.test_weakref.MappingTestCase) ... ok
test_make_weak_keyed_dict_from_weak_keyed_dict 
(test.test_weakref.MappingTestCase) ... ok
test_make_weak_keyed_dict_repr (test.test_weakref.MappingTestCase) ... ok
test_make_weak_valued_dict_from_dict (test.test_weakref.MappingTestCase) ... ok
test_make_weak_valued_dict_from_weak_valued_dict 
(test.test_weakref.MappingTestCase) ... ok
test_make_weak_valued_dict_misc (test.test_weakref.MappingTestCase) ... ok
test_make_weak_valued_dict_repr (test.test_weakref.MappingTestCase) ... ok
test_threaded_weak_valued_pop (test.test_weakref.MappingTestCase) ... Exception 
ignored in: .remove at 
0x7fb20612bb70>
Traceback (most recent call last):
  File "/usr/lib64/python3.7/weakref.py", line 114, in remove
del self.data[wr.key]
KeyError: (10,)
Exception ignored in: .remove at 
0x7fb20612bb70>
Traceback (most recent call last):
  File "/usr/lib64/python3.7/weakref.py", line 114, in remove
del self.data[wr.key]
KeyError: (10,)
Exception ignored in: .remove at 
0x7fb20612bb70>
Traceback (most recent call last):
  File "/usr/lib64/python3.7/weakref.py", line 114, in remove
del self.data[wr.key]
KeyError: (10,)
Exception ignored in: .remove at 
0x7fb20612bb70>
Traceback (most recent call last):
  File "/usr/lib64/python3.7/weakref.py", line 114, in remove
del self.data[wr.key]
KeyError: (10,)
Exception ignored in: .remove at 
0x7fb20612bb70>
Traceback (most recent call last):
  File "/usr/lib64/python3.7/weakref.py", line 114, in remove
del self.data[wr.key]
KeyError: (10,)
Exception ignored in: .remove at 
0x7fb20612bb70>
Traceback (most recent call last):
  File "/usr/lib64/python3.7/weakref.py", line 114, in remove
del self.data[wr.key]
KeyError: (10,)
Exception ignored in: .remove at 
0x7fb20612bb70>
Traceback (most recent call last):
  File "/usr/lib64/python3.7/weakref.py", line 114, in remove
del self.data[wr.key]
KeyError: (10,)
ok
test_threaded_weak_valued_setdefault (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_bad_delitem (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_cascading_deletes (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_delitem (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_dict_popitem (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_dict_setdefault (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_dict_update (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_iters (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_len_cycles (test.test_weakref.MappingTestCase) ... ok
test_weak_keyed_len_race (test.test_weakref.MappingTestCase) ... ok
test_weak_keys (test.test_weakref.MappingTestCase) ... ok
test_weak_keys_destroy_while_iterating (test.test_weakref.MappingTestCase) ... 
ok
test_weak_valued_delitem (test.test_weakref.MappingTestCase) ... ok
test_weak_valued_dict_popitem (test.test_weakref.MappingTestCase) ... ok
test_weak_valued_dict_setdefault (test.test_weakref.MappingTestCase) ... ok
test_weak_valued_dict_update (test.test_weakref.MappingTestCase) ... ok
test_weak_valued_iters (test.test_weakref.MappingTestCase) ... ok
test_weak_valued_len_cycles (test.test_weakref.MappingTestCase) ... ok
test_weak_valued_len_race (test.test_weakref.MappingTestCase) ... ok
test_weak_values (test.test_weakref.MappingTestCase) ... ok
test_weak_values_destroy_while_iterating (test.test_weakref.MappingTestCase) 
... ok
...

--
Ran 116 tests in 3.641s
...

--
nosy: +Arfrever
resolution: fixed -> 
status: closed -> open
versions: +Python 3.7 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Antoine Pitrou]

Antoine Pitrou added the comment:

This is finally fixed!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Roundup Robot]

Roundup Robot added the comment:

New changeset bcb1f0698401 by Antoine Pitrou in branch '2.7':
Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and 
WeakValueDictionary.pop()
https://hg.python.org/cpython/rev/bcb1f0698401

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Roundup Robot]

Roundup Robot added the comment:

New changeset 5a542a2bca08 by Antoine Pitrou in branch '3.5':
Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and 
WeakValueDictionary.pop()
https://hg.python.org/cpython/rev/5a542a2bca08

New changeset f3706a9430da by Antoine Pitrou in branch '3.6':
Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and 
WeakValueDictionary.pop()
https://hg.python.org/cpython/rev/f3706a9430da

New changeset ac2715d04119 by Antoine Pitrou in branch 'default':
Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and 
WeakValueDictionary.pop()
https://hg.python.org/cpython/rev/ac2715d04119

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()()

[Armin Rigo]

Armin Rigo added the comment:

ping

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Antoine Pitrou]

Antoine Pitrou added the comment:

Yes, we need to fix this. Sorry for being a bit slow.

--
versions: +Python 3.5, Python 3.6 -Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Tin Tvrtković]

Tin Tvrtković added the comment:

We're actually getting bitten by this in production through the Riak Python 
client, so this isn't a strictly theoretical problem.

--
nosy: +tinchester

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Armin Rigo]

Armin Rigo added the comment:

Converted the test into an official test, which fails; and wrote the patch for 
CPython 3.trunk and for CPython 2.7.  Please review and commit.

--
keywords: +needs review -patch
stage: needs patch - patch review

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Armin Rigo]

Changes by Armin Rigo ar...@users.sourceforge.net:


Added file: http://bugs.python.org/file37993/fix-weakvaluedict-2.7.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Armin Rigo]

Changes by Armin Rigo ar...@users.sourceforge.net:


--
keywords: +patch
Added file: http://bugs.python.org/file37992/fix-weakvaluedict.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Antoine Pitrou]

Antoine Pitrou added the comment:

Changeset ea70032a24b1 is where the pop(*args) thing comes from.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Ned Deily]

Changes by Ned Deily n...@acm.org:


--
nosy: +fdrake, pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Antoine Pitrou]

Antoine Pitrou added the comment:

I think the underlying question is: are weak dicts otherwise MT-safe?

--
versions: +Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Armin Rigo]

Armin Rigo added the comment:

As you can see in x.py, the underlying question is rather: are weakdicts usable 
in a single thread of a multithreaded program?  I believe that this question 
cannot reasonably be answered No, independently on the answer you want to give 
to your own question.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Antoine Pitrou]

Antoine Pitrou added the comment:

Ah, you're right. We just need to cook up a patch then.

--
stage:  - needs patch
type:  - behavior

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Tim Peters]

Tim Peters added the comment:

The weakref.slice fix looks solid to me, although it appears to be specific to 
2.7 (the methods are fancier on the current default branch, fiddling with 
self._pending_removals too).

Does anyone know why the signature of pop is:

def pop(self, key, *args)

?  It doesn't make sense to me to accept any number of arguments following 
`key`.  Perhaps it's just an obscure way to avoid doing:

_noarg = object()  # unique sentinel

def pop(self, key, default=_noarg):

?

--
nosy: +tim.peters

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Armin Rigo]

New submission from Armin Rigo:

WeakValueDictionary.setdefault() contains a bug that shows up in multithreaded 
situations using reference cycles.  Attached a test case: it is possible for 
'setdefault(key, default)' to return None, although None is never put as a 
value in the dictionary.  (Actually, being a WeakValueDictionary, None is not 
allowed as a value.)

The reason is that the code in setdefault() ends in return wr(), but the 
weakref wr might have gone invalid between the time it was fetched from 
self.data and the wr() itself, thus returning None.

A similar problem occurs in pop(), leading it to possibly raise KeyError even 
if it is called with a default argument.

--
components: Library (Lib)
messages: 202510
nosy: arigo
priority: normal
severity: normal
status: open
title: WeakValueDictionary bug in setdefault()pop()
versions: Python 2.7, Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Armin Rigo]

Changes by Armin Rigo ar...@users.sourceforge.net:


Added file: http://bugs.python.org/file32557/x.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19542] WeakValueDictionary bug in setdefault()pop()

[Armin Rigo]

Changes by Armin Rigo ar...@users.sourceforge.net:


Added file: http://bugs.python.org/file32558/weakref.slice

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19542
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com