Author vstinner
Recipients pablogsal, vstinner, xtreak
Date 2019-02-12.18:24:42
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1549995883.03.0.920789374635.issue35961@roundup.psfhosted.org>
In-reply-to
Content
Extract of slice_richcompare():

    t1 = PyTuple_New(3);
    if (t1 == NULL)
        return NULL;
    t2 = PyTuple_New(3);
    if (t2 == NULL) {
        Py_DECREF(t1);
        return NULL;
    }

    PyTuple_SET_ITEM(t1, 0, ((PySliceObject *)v)->start);
    PyTuple_SET_ITEM(t1, 1, ((PySliceObject *)v)->stop);
    PyTuple_SET_ITEM(t1, 2, ((PySliceObject *)v)->step);
    PyTuple_SET_ITEM(t2, 0, ((PySliceObject *)w)->start);
    PyTuple_SET_ITEM(t2, 1, ((PySliceObject *)w)->stop);
    PyTuple_SET_ITEM(t2, 2, ((PySliceObject *)w)->step);

    res = PyObject_RichCompare(t1, t2, op);

    PyTuple_SET_ITEM(t1, 0, NULL);
    PyTuple_SET_ITEM(t1, 1, NULL);
    PyTuple_SET_ITEM(t1, 2, NULL);
    PyTuple_SET_ITEM(t2, 0, NULL);
    PyTuple_SET_ITEM(t2, 1, NULL);
    PyTuple_SET_ITEM(t2, 2, NULL);

    Py_DECREF(t1);
    Py_DECREF(t2);

    return res;

t1 and t2 tuples are tracked by the GC, but t1 and t2 items are *borrowed* references.

gc.collect() calls subtract_refs() which decreases the reference counter of all objects tracked by the GC... but v, w, t1 and t2 contain the same references which causes the assertion error.

The code is quite old:

commit 47b9ff6ba11fab4c90556357c437cb4feec1e853
Author: Guido van Rossum <guido@python.org>
Date:   Thu Aug 24 00:41:19 2006 +0000

    Restructure comparison dramatically.  There is no longer a default
    *ordering* between objects; there is only a default equality test
    (defined by an object being equal to itself only).  Read the comment
    in object.c.  The current implementation never uses a three-way
    comparison to compute a rich comparison, but it does use a rich
    comparison to compute a three-way comparison.  I'm not quite done
    ripping out all the calls to PyObject_Compare/Cmp, or replacing
    tp_compare implementations with tp_richcompare implementations;
    but much of that has happened (to make most unit tests pass).
    (...)

+static PyObject *
+slice_richcompare(PyObject *v, PyObject *w, int op)
...
+       t1 = PyTuple_New(3);
+       t2 = PyTuple_New(3);
+       if (t1 == NULL || t2 == NULL)
+               return NULL;
+
+       PyTuple_SET_ITEM(t1, 0, ((PySliceObject *)v)->start);
+       PyTuple_SET_ITEM(t1, 1, ((PySliceObject *)v)->stop);
+       PyTuple_SET_ITEM(t1, 2, ((PySliceObject *)v)->step);
+       PyTuple_SET_ITEM(t2, 0, ((PySliceObject *)w)->start);
+       PyTuple_SET_ITEM(t2, 1, ((PySliceObject *)w)->stop);
+       PyTuple_SET_ITEM(t2, 2, ((PySliceObject *)w)->step);
...

I see two options:

* Ensure that t1 and t2 temporary objects are not tracked by the GC
* Modify slice_richcompare() to use strong references, rather than *borrowed* references

I wrote PR 11828 which implements the first solution. The second solution has a minor impact on performance.
History
Date User Action Args
2019-02-12 18:24:43vstinnersetrecipients: + vstinner, pablogsal, xtreak
2019-02-12 18:24:43vstinnersetmessageid: <1549995883.03.0.920789374635.issue35961@roundup.psfhosted.org>
2019-02-12 18:24:43vstinnerlinkissue35961 messages
2019-02-12 18:24:42vstinnercreate