[vtk-developers] Gil tests on megas

[Ben Boeckel]

On Tue, Nov 17, 2015 at 12:53:09 +0100, Mathieu Westphal wrote:
> I'm testing my GIl ensured version of VTK (
> https://gitlab.kitware.com/vtk/vtk/merge_requests/582) with the buildbots,
> and i have only three problematic recurring tests, on a specific build of
> megas.

Found it. Here's the code from PyVTKObject_FromPointer:

>   const char *classname = vtkPythonUtil::StripModule(pytype->tp_name);

...

>     // Use the vtkname of the supplied class type
>     PyObject *s = PyObject_GetAttrString((PyObject *)pytype, "__vtkname__");
>     if (s)
>       {
> #ifdef VTK_PY3K
>       PyObject *tmp = PyUnicode_AsUTF8String(s);

Python3 uses unicode objects, but we need ASCII...

>       if (tmp)
>         {
>         Py_DECREF(s);
>         s = tmp;

...so `s` is now a new bytes object...

>         }
> #endif
>       classname = PyBytes_AsString(s);

...we take a pointer to its data for use later...

>       if (classname == 0)
>         {
>         Py_DECREF(s);
>         return NULL;
>         }
>       }
>     cls = vtkPythonUtil::FindClass(classname);
>     if (cls == 0)
>       {
>       PyErr_Format(PyExc_ValueError,
>                    "internal error, unknown VTK class %.200s",
>                    classname);
>       Py_XDECREF(s);
>       return NULL;
>       }
>     Py_XDECREF(s);

...decref the bytes object here (which can now be garbage collected)...

>     }
> 
>   if (!ptr)
>     {
>     // Create a new instance of this class since we were not given one.
>     if (cls->vtk_new)
>       {
>       ptr = cls->vtk_new();
>       if (!ptr)
>         {
>         // The vtk_new() method returns null when a factory class has no
>         // implementation (i.e. cannot provide a concrete class instance.)
>         // NotImplementedError indicates a pure virtual method call.
>         PyErr_SetString(
>           PyExc_NotImplementedError,
>           "no concrete implementation exists for this class");
>         return 0;
>         }
>       created = true;
> 
>       // Check the type of the newly-created object
>       const char *newclassname = ptr->GetClassName();
>       if (strcmp(newclassname, classname) != 0)

...and use the pointer to the old object's data here.

It only crashes in Python3 and when the garbage collector runs between
the Py_XDECREF above and its use here. The test fails because glibc
catches the bogus pointer after free() does its thing, so that's why
only megas caught it.

Branch incoming :) .

--Ben