Crash in python wrapped MRML node on load during enum iterating

Development
,
1

Hello all,

I am experience a crash with a tp_dict is NULL but being accessed when PyvtkMRMLCameraNode_ClassNew is being called.

Code:

PyObject *d = pytype->tp_dict;
  PyObject *o;

  for (int c = 0; c < 2; c++)
  {
    static const struct { const char *name; int value; }
      constants[2] = {
        { "IntrinsicsModifiedEvent", vtkMRMLCameraNode::IntrinsicsModifiedEvent },
        { "DistortionCoefficientsModifiedEvent", vtkMRMLCameraNode::DistortionCoefficientsModifiedEvent },
      };

    o = PyInt_FromLong(constants[c].value);
    if (o)
    {
      PyDict_SetItemString(d, constants[c].name, o); **<-- crash here, d is NULL**
      Py_DECREF(o);
    }
  }

Commenting out the offending line allows Slicer to load as expected with module loaded.

Has anyone seen this before?

Adam

2

I don’t remember seeing this error, but the behavior of the application is undefined if you perform any operations before application’s startupCompleted signal is emitted.

Can you send a link to the source code line that triggers the error?

3

But, presumably, without those enums actually exposed to Python?

I guess those are new enums, added to the class header, because I don’t see them in current trunk. So it would be worth comparing the code generated for other enums in the same class, or in similar generated vtk*Python.cxx classes under Slicer-build. There are many other enums in that class (for example: ::Direction) which should have exactly the same pattern and crash in the same way if .tp_dict is not initialized. The line near the top of vtkX_ClassNew that contains if ((pytype->tp_flags & Py_TPFLAGS_READY) != 0) {return...} is supposed to guard against uninitialized fields like this.

4

@lassoan I can’t, as it’s in the autogenerated python cxx.

The line that crashes is this one:

PyDict_SetItemString(d, constants[c].name, o);

@ihnorton yes, correct, without being able to access the enum (I needed to test other code, so it was a temporary workaround).

It is not new (via commits), but it’s the first time I’ve tried to load the extension in Slicer, so in that sense it’s new.

I am going to compare against other MRML node types and try to figure out how mine is different (which was also @cpinter’s suggestion).

Edit: I’ve commited the latest code

5

Oh, I was looking at the class by the same name in core:

That might be the problem… Maybe a race condition in the initialization where the core object of the same name is created already but not “finalized”, and your initializer crashes trying to use it?

(did you just add BTX around the enums here to fix the crash temporarily?)

1 Like
6

Aaahhahaha.

So, we don’t need to talk about it, but did you guys know that there’s a vtkMRMLCameraNode in the core?

2 Likes
7

So, that was obviously one issue, but I’m still having a crash. Somehow vtkMRMLWebcamNode has already been initialized, so it’s found in the class map, so tp_dict is never new’d.

PyVTKClass *PyVTKClass_Add(
  PyTypeObject *pytype, PyMethodDef *methods,
  const char *classname, vtknewfunc constructor)
{
  // Add this type to the vtk class map
  PyVTKClass *info =
    vtkPythonUtil::AddClassToMap(
      pytype, methods, classname, constructor);

  if (info == nullptr)
  {
    // The class was already in the map, so do nothing
    return info;
  }

  // Cache the type object for vtkObjectBase for quick access
  if (PyVTKObject_Type == nullptr && strcmp(classname, "vtkObjectBase") == 0)
  {
    PyVTKObject_Type = pytype;
  }

  // Create the dict
  if (pytype->tp_dict == nullptr)
  {
    pytype->tp_dict = PyDict_New();
  }

info is always null, even if this is the first time my code gets hit. Investigating further.

8

One thing that sticks out to me is that the enums are declared as the very first members in the class (as the code currently appears on github). I think most Slicer classes have the static constructor and vtkTypeMacro stuff first. The wrapper-generator might be generating some initialization stanza out of order (it seems to be very straight-line codegen, not really contextual).

9

I think it’s a clean-build issue. Deleted folder of old naming convention, cleared symbol cache in VS, see if it works.