Merge pull request #1142 from undingen/misc_refcounting_fixed2

Misc refcounting leaks encountered while fixing the generator abandonment tests part 2

from_cpython/Lib/test/test_parser.py

-# expected: reffail
-# - leaked refs
 import parser
 import unittest
 import sys

from_cpython/Lib/test/test_structseq.py

-# expected: reffail
-# - unknown segfault
 import unittest
 from test import test_support
 

from_cpython/Modules/parsermodule.c

@@ -3477,7 +3477,7 @@ initparser(void)
         PyObject *func, *pickler;
 
         func = PyObject_GetAttrString(copyreg, "pickle");
-        pickle_constructor = PyObject_GetAttrString(module, "sequence2st");
+        pickle_constructor = PyGC_RegisterStaticConstant(PyObject_GetAttrString(module, "sequence2st"));
         pickler = PyObject_GetAttrString(module, "_pickler");
         Py_XINCREF(pickle_constructor);
         if ((func != NULL) && (pickle_constructor != NULL)

src/runtime/float.cpp

@@ -1122,7 +1122,7 @@ static Box* floatConjugate(Box* b, void*) {
         raiseExcHelper(TypeError, "descriptor 'conjugate' requires a 'float' object but received a '%s'",
                        getTypeName(b));
     if (b->cls == float_cls) {
-        return b;
+        return incref(b);
     } else {
         assert(PyFloat_Check(b));
         return boxFloat(static_cast<BoxedFloat*>(b)->d);

src/runtime/iterators.cpp

@@ -19,8 +19,6 @@
 namespace pyston {
 namespace {
 
-static std::string next_str("next");
-
 class BoxIteratorGeneric : public BoxIteratorImpl {
 private:
     Box* iterator;
@@ -31,9 +29,17 @@ public:
         if (container) {
             // TODO: this should probably call getPystonIter
             iterator = getiter(container);
-            if (iterator)
-                next();
-            else
+            if (iterator) {
+                // try catch block to manually decref the iterator because if the constructor throwes the destructor
+                // won't get called
+                // but we should probably just change the code to not call next inside the constructor...
+                try {
+                    next();
+                } catch (ExcInfo e) {
+                    Py_CLEAR(iterator);
+                    throw e;
+                }
+            } else
                 *this = *end();
         }
     }

src/runtime/iterobject.cpp

@@ -116,7 +116,7 @@ Box* seqiter_next(Box* s) noexcept {
             hasnext = seqreviterHasnext_capi(s);
         else
             RELEASE_ASSERT(0, "");
-        AUTO_DECREF(hasnext);
+        AUTO_XDECREF(hasnext);
         if (hasnext != True)
             return NULL;
     }

src/runtime/set.cpp

@@ -24,7 +24,7 @@ extern "C" Box* createSet() {
     return new BoxedSet();
 }
 
-void _setAddStolen(BoxedSet* self, STOLEN(BoxAndHash) val) {
+static void _setAddStolen(BoxedSet* self, STOLEN(BoxAndHash) val) {
     auto&& p = self->s.insert(val);
     if (!p.second /* already exists */) {
         // keep the original key
@@ -32,6 +32,13 @@ void _setAddStolen(BoxedSet* self, STOLEN(BoxAndHash) val) {
     }
 }
 
+void _setAddStolen(BoxedSet* self, STOLEN(Box*) val) {
+    AUTO_DECREF(val);
+    BoxAndHash val_hashed(val); // this can throw!
+    incref(val_hashed);
+    _setAddStolen(self, val_hashed);
+}
+
 namespace set {
 
 class BoxedSetIterator : public Box {
@@ -118,8 +125,8 @@ BoxedSet* makeNewSet(BoxedClass* cls, Box* container) {
     assert(isSubclass(cls, frozenset_cls) || isSubclass(cls, set_cls));
 
     BoxedSet* rtn = new (cls) BoxedSet();
-
     if (container) {
+        AUTO_DECREF(rtn);
         if (PyAnySet_Check(container)) {
             for (auto&& elt : ((BoxedSet*)container)->s) {
                 rtn->s.insert(incref(elt));
@@ -133,6 +140,7 @@ BoxedSet* makeNewSet(BoxedClass* cls, Box* container) {
                 _setAddStolen(rtn, elt);
             }
         }
+        return incref(rtn);
     }
     return rtn;
 }
@@ -303,14 +311,14 @@ static BoxedSet* setIntersection2(BoxedSet* self, Box* container) {
     RELEASE_ASSERT(PyAnySet_Check(self), "");
 
     BoxedSet* rtn = makeNewSet(self->cls, NULL);
+    AUTO_DECREF(rtn);
     for (auto elt : container->pyElements()) {
-        if (self->s.count(elt)) {
-            _setAddStolen(rtn, elt);
-        } else {
-            Py_DECREF(elt);
-        }
+        AUTO_DECREF(elt);
+        BoxAndHash elt_hashed(elt); // this can throw!
+        if (self->s.count(elt_hashed))
+            _setAdd(rtn, elt_hashed);
     }
-    return rtn;
+    return incref(rtn);
 }
 
 static Box* setIntersectionUpdate2(BoxedSet* self, Box* other) {
@@ -480,6 +488,8 @@ Box* setUnion(BoxedSet* self, BoxedTuple* args) {
         raiseExcHelper(TypeError, "descriptor 'union' requires a 'set' object but received a '%s'", getTypeName(self));
 
     BoxedSet* rtn = makeNewSet(self->cls, self);
+    AUTO_DECREF(rtn);
+
     for (auto&& p : self->s)
         _setAdd(rtn, p);
 
@@ -489,8 +499,7 @@ Box* setUnion(BoxedSet* self, BoxedTuple* args) {
             _setAddStolen(rtn, elt);
         }
     }
-
-    return rtn;
+    return incref(rtn);
 }
 
 static void _setDifferenceUpdate(BoxedSet* self, BoxedTuple* args) {
@@ -528,8 +537,9 @@ Box* setDifference(BoxedSet* self, BoxedTuple* args) {
                        getTypeName(self));
 
     BoxedSet* rtn = makeNewSet(self->cls, self);
+    AUTO_DECREF(rtn);
     _setDifferenceUpdate(rtn, args);
-    return rtn;
+    return incref(rtn);
 }
 
 Box* setSymmetricDifferenceUpdate(BoxedSet* self, Box* other) {
@@ -548,8 +558,9 @@ Box* setSymmetricDifference(BoxedSet* self, Box* other) {
                        getTypeName(self));
 
     BoxedSet* rtn = makeNewSet(self->cls, self);
+    AUTO_DECREF(rtn);
     _setSymmetricDifferenceUpdate(rtn, other);
-    return rtn;
+    return incref(rtn);
 }
 
 static Box* setIssubset(BoxedSet* self, Box* container) {

src/runtime/set.h

@@ -42,7 +42,7 @@ public:
     static int clear(Box* self) noexcept;
 };
 
-void _setAddStolen(BoxedSet* self, STOLEN(BoxAndHash) val);
+void _setAddStolen(BoxedSet* self, STOLEN(Box*) val);
 }
 
 #endif

src/runtime/tuple.cpp

@@ -116,29 +116,53 @@ extern "C" int _PyTuple_Resize(PyObject** pv, Py_ssize_t newsize) noexcept {
 }
 
 int BoxedTuple::Resize(BoxedTuple** pv, size_t newsize) noexcept {
-    assert((*pv)->cls == tuple_cls);
-
-    BoxedTuple* t = static_cast<BoxedTuple*>(*pv);
-    size_t oldsize = t->size();
-
-    if (newsize == oldsize)
-        return 0;
+    // cpythons _PyTuple_Resize with small s/PyTupleObject/BoxedTuple modifications
+    BoxedTuple* v;
+    BoxedTuple* sv;
+    Py_ssize_t i;
+    Py_ssize_t oldsize;
 
-    if (newsize < oldsize) {
-        // XXX resize the box (by reallocating) smaller if it makes sense
-        t->ob_size = newsize;
-        for (int i = newsize; i < oldsize; i++) {
-            Py_CLEAR((*pv)->elts[i]);
-        }
-        return 0;
+    v = *pv;
+    if (v == NULL || v->cls != &PyTuple_Type || (Py_SIZE(v) != 0 && Py_REFCNT(v) != 1)) {
+        *pv = 0;
+        Py_XDECREF(v);
+        PyErr_BadInternalCall();
+        return -1;
     }
+    oldsize = Py_SIZE(v);
+    if (oldsize == newsize)
+        return 0;
 
-
-    BoxedTuple* resized = new (newsize) BoxedTuple();
-    memmove(resized->elts, t->elts, sizeof(Box*) * oldsize);
-    memset(resized->elts + oldsize, 0, sizeof(Box*) * (newsize - oldsize));
-
-    *pv = resized;
+    if (oldsize == 0) {
+        /* Empty tuples are often shared, so we should never
+           resize them in-place even if we do own the only
+           (current) reference */
+        Py_DECREF(v);
+        *pv = (BoxedTuple*)PyTuple_New(newsize);
+        return *pv == NULL ? -1 : 0;
+    }
+
+    /* XXX UNREF/NEWREF interface should be more symmetrical */
+    _Py_DEC_REFTOTAL;
+    if (_PyObject_GC_IS_TRACKED(v))
+        _PyObject_GC_UNTRACK(v);
+    _Py_ForgetReference((PyObject*)v);
+    /* DECREF items deleted by shrinkage */
+    for (i = newsize; i < oldsize; i++) {
+        Py_CLEAR(v->elts[i]);
+    }
+    sv = PyObject_GC_Resize(BoxedTuple, v, newsize);
+    if (sv == NULL) {
+        *pv = NULL;
+        PyObject_GC_Del(v);
+        return -1;
+    }
+    _Py_NewReference((PyObject*)sv);
+    /* Zero out items added by growing */
+    if (newsize > oldsize)
+        memset(&sv->elts[oldsize], 0, sizeof(*sv->elts) * (newsize - oldsize));
+    *pv = sv;
+    _PyObject_GC_TRACK(sv);
     return 0;
 }
 

src/runtime/types.cpp

@@ -2425,8 +2425,9 @@ public:
 
         Box* r = self->b->getattr(key);
         if (r) {
+            Py_INCREF(r);
             self->b->delattr(key, NULL);
-            return incref(r);
+            return r;
         } else {
             if (default_)
                 return incref(default_);

test/tests/float.py

@@ -69,6 +69,7 @@ print (0.5).as_integer_ratio()
 print (0.5).is_integer()
 print (1.0).is_integer()
 print 1.0.__hash__(), 1.1.__hash__(), -1.1.__hash__()
+print (0.5).conjugate(), (0.6).imag
 
 print 1.0 ** (10 ** 100)
 print (-1.0) ** (10 ** 100)

test/tests/tuples.py

@@ -260,3 +260,6 @@ try:
     print((1, 2) * d)
 except TypeError as e:
     print(e.message)
+
+# this triggers a tuple resize because generators have a unknown len:
+print len(tuple(v*10 for v in range(100)))