0

Working on a control of an eGige camera, having a C library, I've started a cython code project with the idea to have the best things of each language.

The library provides a way to listen a heartbeat from the camera to know if it has been disconnected. The callback with in a C++ class I've already did, but from this C++ class, call a python method of a class is falling into a Segmentation fault in all the ways I've tried.

I've encapsulated it in a specific C++ class:

#include <Python.h>
/* (...) */
PyCallback::PyCallback(PyObject* self, const char* methodName)
{
  Py_XINCREF(self);
  _self = self;
  _method = PyObject_GetAttrString(self, methodName);
}
PyCallback::~PyCallback()
{
  Py_XDECREF(_self);
}
void PyCallback::execute()
{
  try
  {
    PyObject *args = PyTuple_Pack(1,_self);
    PyObject_CallFunctionObjArgs(_method, args);
  }catch(...){
    _error("Exception calling python");
  }
}

From a cython object the code is:

cdef class Camera(...):
    # (...)
    cdef registerRemovalCallback(self):
        cdef:
            PyCallback* obj
        obj = new PyCallback(<PyObject*> self, <char*> "cameraRemovalCallback")
    cdef cameraRemovalCallback(self):
        self._isPresent = False

The lowest part of the backtrace it's just when try to prepare the arguments.

#0  0x00007ffff7b24592 in PyErr_Restore () from /usr/lib64/libpython2.6.so.1.0
#1  0x00007ffff7b23fef in PyErr_SetString () from /usr/lib64/libpython2.6.so.1.0
#2  0x00007ffff7b314dd in ?? () from /usr/lib64/libpython2.6.so.1.0
#3  0x00007ffff7b313ca in ?? () from /usr/lib64/libpython2.6.so.1.0
#4  0x00007ffff7b316c1 in ?? () from /usr/lib64/libpython2.6.so.1.0
#5  0x00007ffff7b31d2f in ?? () from /usr/lib64/libpython2.6.so.1.0
#6  0x00007ffff7b31e9c in Py_BuildValue () from /usr/lib64/libpython2.6.so.1.0
#7  0x00007ffff637cbf8 in PyCallback::execute (this=0x16212a0) at pylon/PyCallback.cpp:53
#8  0x00007ffff6376248 in CppCamera::removalCallback (this=0x161fb30, pDevice=<value optimized out>) at pylon/Camera.cpp:387

I've tried to make the arguments using _Py_BuildValue("(self)", self); but then I've got a segfault there.

I've tried also with PyObject_CallFunctionObjArgs with NULL in the arguments field, thinking that perhaps the pointer to "self" is already embedded as the method point to an specific address with in this object. But them I've got the segfault there.

Do anyone see my mistake? Something there that shall be made in a different way? I hope this is a misunderstanding from my side about who to do that.

Update @ 2016/08/01:

Following comments indications, two modifications are made in the code:

First of all the pointer storage to the PyCallback has been stored as a member of the Camera cython class:

cdef class Camera(...):
    cdef:
        #(...)
        PyCallback* _cbObj
    # (...)
    cdef registerRemovalCallback(self):
        self._cbObj = new PyCallback(<PyObject*> self, <char*> "cameraRemovalCallback")
    cdef cameraRemovalCallback(self):
        self._isPresent = False

Even this is a fundamental source of segfaults it looks it wasn't involved in the current one.

Then PyCallback::execute() in the c++, I've made some changes. After reading about the GIL (Global Interpreter Lock) and add some calls for it, I've added a check that may guide to the solution:

PyCallback::PyCallback(PyObject* self, const char* methodName)
{
  Py_Initialize();
  Py_XINCREF(self);
  _self = self;
  _method = PyObject_GetAttrString(self, methodName);
}

PyCallback::~PyCallback()
{
  Py_XDECREF(_self);
  Py_Finalize();
}

void PyCallback::execute()
{
  PyGILState_STATE gstate;

  gstate = PyGILState_Ensure();
  try
  {
    if ( PyCallable_Check(_method) )
    {
      _info("Build arguments and call method");
      PyObject *args = Py_BuildValue("(O)", _self);
      PyObject *kwargs = Py_BuildValue("{}", "", NULL);
      PyObject_Call(_method, args, kwargs);
    }
    else
    {
      _warning("The given method is not callable!");
    }
  }
  catch(...)
  {
    // TODO: collect and show more information about the exception
    _error("Exception calling python");
  }
  PyGILState_Release(gstate);
}

Even I'm not sure how to do the call, the main point is that the _PyCallable_Check_ returns false.

I've also tested to use the typedef option and C pointer to function to call it with the same segfault result.

Update @ 2016/08/03:

I've proceed with the suggested modifications. cameraRemovalCallback is now changed from cdef to def and some ifs in the PyCallback reports that the method now is found. Also has been added to ~PyCallback() a call to Py_XDECREF(_method) in case it was found in the constructor. The useless try-catch has been also removed.

From the reference to the Python's Object protocol, that DavidW mention, I've check many of the *Call*combinations: falling to the segfault.

I think this question is becoming dirty and is getting the appearance of a forum (question->answer->replay->...). I'm sorry about that, and I'll try to, next time I'll write, tell the segfault was solve and just what was.

Community
  • 1
  • 1
srgblnch
  • 323
  • 3
  • 11
  • 1
    Callback functions can be implemented in cython code. See [a callback example](https://github.com/cython/cython/blob/master/Demos/callback/cheese.pyx), [an old question](http://stackoverflow.com/questions/5242051/cython-implementing-callbacks), and [an old question](http://stackoverflow.com/questions/11700501/python-cython-c-and-callbacks-calling-a-python-function-from-c-using-cython). – J.J. Hakala Jul 29 '16 at 07:57
  • 1
    The documentation for `PyObject_CallFunctionObjArgs` https://docs.python.org/2/c-api/object.html#c.PyObject_CallFunctionObjArgs implies that you should pass a variable number of `PyObject*`s followed by `NULL`. The `NULL` is important because it tells Python that the list of args is overs For example `PyObject_CallFunctionObjArgs(_method, self, NULL);` – DavidW Jul 29 '16 at 19:39
  • 1
    Unfortunately, your example isn't complete enough to tell if that's the only problem. (Also: at least in the code provided, `obj` isn't actually saved anywhere in `registerRemovalCallback`.) – DavidW Jul 29 '16 at 19:42
  • Thanks for the comments. I've seen the example and the other questions. But they were the first tries I did until I thought to use the c-api. About the second coment, you are right and I did a mesh with tries. But the third one looks to be a great candidate, because you are right and I'm not storing the pointer. Often is the source of many segfaults. – srgblnch Jul 29 '16 at 20:12

1 Answers1

1

I'm not promising this is the only issue, but this is certainly an issue:

cameraRemovalCallback is a cdef function. This means that the function is purely accessible from C/Cython, but not accessible from Python. This means that PyObject_GetAttrString fails (since cameraRemovalCallback is not a Python attribute).

You should define cameraRemovalCallback with def instead of cdef and that was it will be accessible through normal Python mechanisms. You should also check the result of PyObject_GetAttrString - if it returns NULL then it has failed to find the attribute.

Therefore you end up trying to call NULL as a Python function.


Other smaller issues:

You should decref _method in ~PyCallback.

You should not call Py_Initialize and Py_Finalize. You seem to be creating the class from within Python anyway so it doesn't need initializing or finalizing. Finalizing will definitely cause you problems.

I don't think you need to pass self as an argument to PyObject_Call. (I could be wrong though)

The Python C api won't raise C++ exceptions, so your try{} catch(...) is never going to catch anything. Instead check the return values.

You need to decref the results of both calls of Py_BuildValue (when you're done with them) also the result of PyObject_Call. If you don't do this you're leaking memory.


The following complete example works for me (with Python 3.5 - I can't easily test it with earlier versions). If it works for you then you probably need to identify what's different in your case? If it doesn't work for you then it's more mysterious.

pycallback.hpp:

#include <Python.h>
#include <stdexcept>

inline PyObject* getCallable(PyObject* o, const char* methodName) {
    // assume o is not null
    PyObject* callable = PyObject_GetAttrString(o,methodName);
    if (callable == nullptr) {
        throw std::runtime_error("Attribute does not exist");
    }
    return callable;
}

class PyCallback {
private:
    PyObject* _callable;

public:
    PyCallback(PyObject* callable) {
        // assume callable isn't null
        if (!PyCallable_Check(callable)) {
            throw std::runtime_error("object passed to PyCallback is not callable");
        }
        _callable = callable;
        Py_XINCREF(_callable);
    }

    PyCallback(PyObject* o, const char* methodName) : 
    PyCallback(getCallable(o,methodName)) { // needs C++11 to compile
    }

    // don't define copy operators
    PyCallback(const PyCallback&) = delete;
    PyCallback& operator=(const PyCallback&) = delete;

    ~PyCallback() {
        Py_XDECREF(_callable);
    }

    void execute() {
        PyGILState_STATE gstate;
        gstate = PyGILState_Ensure();

        PyObject* result = PyObject_CallFunctionObjArgs(_callable,nullptr);

        Py_XDECREF(result); // allowed to be null
        PyGILState_Release(gstate);
    }
};

camera.pyx

cdef extern from "pycallback.hpp":
    cdef cppclass PyCallback:
        PyCallback(object) except +
        PyCallback(object, const char*) except +
        void execute()

cdef class Camera:
    cdef PyCallback* o
    cdef public ispresent

    def __init__(self):
        self.o = NULL
        self.ispresent = True

    def registerRemovalCallback(self):
        self.o = new PyCallback(self,'cameraRemovalCallback')
        #self.o = new PyCallback(self.cameraRemovalCallback)

    def cameraRemovalCallback(self):
        self.ispresent = False

    def triggerCallback(self):
        if self.o != NULL:
            self.o.execute()

setup.py

from distutils.core import setup
from distutils.extension import Extension
from Cython.Distutils import build_ext

setup(
    ext_modules = [
        Extension('camera',sources=["camera.pyx"],
            language="c++",
            extra_compile_args=['-std=c++11'])],
    cmdclass={'build_ext': build_ext})

test.py

import camera

c = camera.Camera()
print(c.ispresent)
c.triggerCallback()
print(c.ispresent)
c.registerRemovalCallback()
print(c.ispresent)
c.triggerCallback()
print(c.ispresent)

Note - there is a minor problem with this. Camera and the callback it holds form a reference loop so they never get freed. This causes a small memory leak, but it won't cause a segmentation fault.

DavidW
  • 29,336
  • 6
  • 55
  • 86
  • I appreciate the issues you've pointed and I've modified the code in the way you've mention. But still a segfault just when I try to call the python method. – srgblnch Aug 03 '16 at 09:34
  • @srgblnch I did add a semi-"minimal" example which seems to work for me (and isn't very different from your code). I suspect this doesn't actually solve your problem but it suggests to me that your problem lies deeper inside your code than the bit you've shown here. [Comment added because I realize that askers don't actually get notified about edits to answers] – DavidW Aug 05 '16 at 12:48
  • Very many thanks, you've got it. I've start reading and comparing what you propose. The last mistake I was making was with the GIL. I've removed together with the `Py_Initialize` thinking they go together. I finaly saw the log message in python when a camera is unplugged and reported from the gige api. I really appreciate the effort you made. – srgblnch Aug 05 '16 at 13:59