From: Antoine Pitrou <solipsis@pitrou.net>
Date: Tue, 27 Dec 2016 13:34:54 +0000 (+0100)
Subject: Issue #28427: old keys should not remove new values from
X-Git-Tag: v3.7.0a1~1675
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c06ae208ebd55ff261438385c4179023f2df0388;p=python

Issue #28427: old keys should not remove new values from
WeakValueDictionary when collecting from another thread.
---

c06ae208ebd55ff261438385c4179023f2df0388
diff --cc Misc/NEWS
index a80a3db8d4,767f3e3b83..b5ec17e30d
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@@ -208,8 -40,14 +208,11 @@@ Core and Builtin
  Library
  -------
  
+ - Issue #28427: old keys should not remove new values from
+   WeakValueDictionary when collecting from another thread.
+ 
  - Issue 28923: Remove editor artifacts from Tix.py.
  
 -- Issue #29055:  Neaten-up empty population error on random.choice()
 -  by suppressing the upstream exception.
 -
  - Issue #28871: Fixed a crash when deallocate deep ElementTree.
  
  - Issue #19542: Fix bugs in WeakValueDictionary.setdefault() and
diff --cc Objects/dictobject.c
index 6a7e831944,64941937e7..baef589427
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@@ -1583,6 -1591,30 +1583,28 @@@ _PyDict_SetItem_KnownHash(PyObject *op
      return insertdict(mp, key, hash, value);
  }
  
+ static int
+ delitem_common(PyDictObject *mp, Py_ssize_t hashpos, Py_ssize_t ix,
 -               PyObject **value_addr)
++               PyObject *old_value)
+ {
 -    PyObject *old_key, *old_value;
++    PyObject *old_key;
+     PyDictKeyEntry *ep;
+ 
 -    old_value = *value_addr;
 -    assert(old_value != NULL);
 -    *value_addr = NULL;
+     mp->ma_used--;
+     mp->ma_version_tag = DICT_NEXT_VERSION();
+     ep = &DK_ENTRIES(mp->ma_keys)[ix];
+     dk_set_index(mp->ma_keys, hashpos, DKIX_DUMMY);
+     ENSURE_ALLOWS_DELETIONS(mp);
+     old_key = ep->me_key;
+     ep->me_key = NULL;
++    ep->me_value = NULL;
+     Py_DECREF(old_key);
+     Py_DECREF(old_value);
+ 
+     assert(_PyDict_CheckConsistency(mp));
+     return 0;
+ }
+ 
  int
  PyDict_DelItem(PyObject *op, PyObject *key)
  {
@@@ -1604,8 -1635,7 +1625,7 @@@ _PyDict_DelItem_KnownHash(PyObject *op
  {
      Py_ssize_t hashpos, ix;
      PyDictObject *mp;
-     PyDictKeyEntry *ep;
-     PyObject *old_key, *old_value;
 -    PyObject **value_addr;
++    PyObject *old_value;
  
      if (!PyDict_Check(op)) {
          PyErr_BadInternalCall();
@@@ -1628,26 -1658,63 +1648,64 @@@
          if (dictresize(mp, DK_SIZE(mp->ma_keys))) {
              return -1;
          }
 -        ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &value_addr, &hashpos);
 +        ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &old_value, &hashpos);
          assert(ix >= 0);
      }
 -    return delitem_common(mp, hashpos, ix, value_addr);
 +
-     assert(old_value != NULL);
-     mp->ma_used--;
-     mp->ma_version_tag = DICT_NEXT_VERSION();
-     ep = &DK_ENTRIES(mp->ma_keys)[ix];
-     dk_set_index(mp->ma_keys, hashpos, DKIX_DUMMY);
-     ENSURE_ALLOWS_DELETIONS(mp);
-     old_key = ep->me_key;
-     ep->me_key = NULL;
-     ep->me_value = NULL;
-     Py_DECREF(old_key);
-     Py_DECREF(old_value);
++    return delitem_common(mp, hashpos, ix, old_value);
+ }
  
-     assert(_PyDict_CheckConsistency(mp));
-     return 0;
+ /* This function promises that the predicate -> deletion sequence is atomic
+  * (i.e. protected by the GIL), assuming the predicate itself doesn't
+  * release the GIL.
+  */
+ int
+ _PyDict_DelItemIf(PyObject *op, PyObject *key,
+                   int (*predicate)(PyObject *value))
+ {
+     Py_ssize_t hashpos, ix;
+     PyDictObject *mp;
+     Py_hash_t hash;
 -    PyObject **value_addr;
++    PyObject *old_value;
+     int res;
+ 
+     if (!PyDict_Check(op)) {
+         PyErr_BadInternalCall();
+         return -1;
+     }
+     assert(key);
+     hash = PyObject_Hash(key);
+     if (hash == -1)
+         return -1;
+     mp = (PyDictObject *)op;
 -    ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &value_addr, &hashpos);
++    ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &old_value, &hashpos);
+     if (ix == DKIX_ERROR)
+         return -1;
 -    if (ix == DKIX_EMPTY || *value_addr == NULL) {
++    if (ix == DKIX_EMPTY || old_value == NULL) {
+         _PyErr_SetKeyError(key);
+         return -1;
+     }
+     assert(dk_get_index(mp->ma_keys, hashpos) == ix);
+ 
+     // Split table doesn't allow deletion.  Combine it.
+     if (_PyDict_HasSplitTable(mp)) {
+         if (dictresize(mp, DK_SIZE(mp->ma_keys))) {
+             return -1;
+         }
 -        ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &value_addr, &hashpos);
++        ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &old_value, &hashpos);
+         assert(ix >= 0);
+     }
+ 
 -    res = predicate(*value_addr);
++    res = predicate(old_value);
+     if (res == -1)
+         return -1;
+     if (res > 0)
 -        return delitem_common(mp, hashpos, ix, value_addr);
++        return delitem_common(mp, hashpos, ix, old_value);
+     else
+         return 0;
  }
  
+ 
  void
  PyDict_Clear(PyObject *op)
  {