Issue #16453: Fix equality testing of dead weakref objects.

Also add tests for hashing.

diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
index f2e328b9eb960f523a1f1c0d22775294eb77eb78..b7c72bd5e1b063409124f65e4a09d862c295d1e6 100644 (file)
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -33,6 +33,27 @@ def create_unbound_method():
     return C.method
 
 
+class Object:
+    def __init__(self, arg):
+        self.arg = arg
+    def __repr__(self):
+        return "<Object %r>" % self.arg
+    def __eq__(self, other):
+        if isinstance(other, Object):
+            return self.arg == other.arg
+        return NotImplemented
+    def __ne__(self, other):
+        if isinstance(other, Object):
+            return self.arg != other.arg
+        return NotImplemented
+    def __hash__(self):
+        return hash(self.arg)
+
+class RefCycle:
+    def __init__(self):
+        self.cycle = self
+
+
 class TestBase(unittest.TestCase):
 
     def setUp(self):
@@ -705,6 +726,54 @@ class ReferencesTestCase(TestBase):
         self.assertEqual(b(), None)
         self.assertEqual(l, [a, b])
 
+    def test_equality(self):
+        # Alive weakrefs defer equality testing to their underlying object.
+        x = Object(1)
+        y = Object(1)
+        z = Object(2)
+        a = weakref.ref(x)
+        b = weakref.ref(y)
+        c = weakref.ref(z)
+        d = weakref.ref(x)
+        # Note how we directly test the operators here, to stress both
+        # __eq__ and __ne__.
+        self.assertTrue(a == b)
+        self.assertFalse(a != b)
+        self.assertFalse(a == c)
+        self.assertTrue(a != c)
+        self.assertTrue(a == d)
+        self.assertFalse(a != d)
+        del x, y, z
+        gc.collect()
+        for r in a, b, c:
+            # Sanity check
+            self.assertIs(r(), None)
+        # Dead weakrefs compare by identity: whether `a` and `d` are the
+        # same weakref object is an implementation detail, since they pointed
+        # to the same original object and didn't have a callback.
+        # (see issue #16453).
+        self.assertFalse(a == b)
+        self.assertTrue(a != b)
+        self.assertFalse(a == c)
+        self.assertTrue(a != c)
+        self.assertEqual(a == d, a is d)
+        self.assertEqual(a != d, a is not d)
+
+    def test_hashing(self):
+        # Alive weakrefs hash the same as the underlying object
+        x = Object(42)
+        y = Object(42)
+        a = weakref.ref(x)
+        b = weakref.ref(y)
+        self.assertEqual(hash(a), hash(42))
+        del x, y
+        gc.collect()
+        # Dead weakrefs:
+        # - retain their hash is they were hashed when alive;
+        # - otherwise, cannot be hashed.
+        self.assertEqual(hash(a), hash(42))
+        self.assertRaises(TypeError, hash, b)
+
 
 class SubclassableWeakrefTestCase(TestBase):
 
@@ -809,17 +878,6 @@ class SubclassableWeakrefTestCase(TestBase):
         self.assertEqual(self.cbcalled, 0)
 
 
-class Object:
-    def __init__(self, arg):
-        self.arg = arg
-    def __repr__(self):
-        return "<Object %r>" % self.arg
-
-class RefCycle:
-    def __init__(self):
-        self.cycle = self
-
-
 class MappingTestCase(TestBase):
 
     COUNT = 10

diff --git a/Misc/NEWS b/Misc/NEWS
index d5a048e2bbf30448a1e736f54b7ffdd395b0c2fb..6f1d66e2a9147b3f05b2c4303a7b59f28a222fb8 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -9,6 +9,8 @@ What's New in Python 2.7.4
 Core and Builtins
 -----------------
 
+- Issue #16453: Fix equality testing of dead weakref objects.
+
 - Issue #9535: Fix pending signals that have been received but not yet
   handled by Python to not persist after os.fork() in the child process.
 

diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
index 0e46d92577f0c7ffffe1a9876518faa08f759cb2..7b943b0f00491bde7a2a88a2d6802d92d59dd5f5 100644 (file)
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -187,15 +187,19 @@ weakref_repr(PyWeakReference *self)
 static PyObject *
 weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op)
 {
-    if (op != Py_EQ || self->ob_type != other->ob_type) {
+    if ((op != Py_EQ && op != Py_NE) || self->ob_type != other->ob_type) {
         Py_INCREF(Py_NotImplemented);
         return Py_NotImplemented;
     }
     if (PyWeakref_GET_OBJECT(self) == Py_None
         || PyWeakref_GET_OBJECT(other) == Py_None) {
-        PyObject *res = self==other ? Py_True : Py_False;
-        Py_INCREF(res);
-        return res;
+        int res = (self == other);
+        if (op == Py_NE)
+            res = !res;
+        if (res)
+            Py_RETURN_TRUE;
+        else
+            Py_RETURN_FALSE;
     }
     return PyObject_RichCompare(PyWeakref_GET_OBJECT(self),
                                 PyWeakref_GET_OBJECT(other), op);