Bug 228 - pybindgen ns3.Object subclassing bug
pybindgen ns3.Object subclassing bug
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: core
pre-release
All All
: P3 normal
Assigned To: Gustavo J. A. M. Carneiro
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-19 10:06 UTC by Gustavo J. A. M. Carneiro
Modified: 2008-07-01 13:32 UTC (History)
1 user (show)

See Also:


Attachments
patch (3.98 KB, patch)
2008-06-19 17:19 UTC, Gustavo J. A. M. Carneiro
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gustavo J. A. M. Carneiro 2008-06-19 10:06:50 UTC
I have to fix this in the python bindings.  Needs to do some custom CreateObject code.

PS: I could a 'python' component' in bugzilla
PPS: whatever happened to the launchpad migration?



>>> import ns3
>>> class My(ns3.Object):pass
... 
>>> My()

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ff66b65b6e0 (LWP 16590)]
0x0000000000448be9 in PyObject_HasAttrString ()
(gdb) up
#1  0x00007ff669bcdcb7 in PyNs3Object__PythonHelper::GetInstanceTypeId (this=0x7dbba0)
    at debug/bindings/python/ns3module.cc:19465
19465	    if (!PyObject_HasAttrString(m_pyself, "_GetInstanceTypeId")) {
Current language:  auto; currently c++
(gdb) p m_pyself
$1 = (PyObject *) 0x0
(gdb) l
19460	{
19461	    ns3::Object *self_obj_before;
19462	    PyObject *py_retval;
19463	    PyNs3TypeId *tmp_TypeId;
19464	
19465	    if (!PyObject_HasAttrString(m_pyself, "_GetInstanceTypeId")) {
19466	        PyErr_Print();
19467	        return ns3::Object::GetInstanceTypeId();
19468	    }
19469	    self_obj_before = reinterpret_cast< PyNs3Object* >(m_pyself)->obj;
(gdb) l -
19450	
19451	    ns3::TypeId retval = reinterpret_cast< PyNs3Object__PythonHelper* >(self->obj)->GetInstanceTypeId__parent_caller();
19452	    py_TypeId = PyObject_New(PyNs3TypeId, &PyNs3TypeId_Type);
19453	    py_TypeId->obj = new ns3::TypeId(retval);
19454	    py_retval = Py_BuildValue("N", py_TypeId);
19455	    return py_retval;
19456	}
19457	
19458	ns3::TypeId
19459	PyNs3Object__PythonHelper::GetInstanceTypeId() const
(gdb) l
19460	{
19461	    ns3::Object *self_obj_before;
19462	    PyObject *py_retval;
19463	    PyNs3TypeId *tmp_TypeId;
19464	
19465	    if (!PyObject_HasAttrString(m_pyself, "_GetInstanceTypeId")) {
19466	        PyErr_Print();
19467	        return ns3::Object::GetInstanceTypeId();
19468	    }
19469	    self_obj_before = reinterpret_cast< PyNs3Object* >(m_pyself)->obj;
(gdb) bt
#0  0x0000000000448be9 in PyObject_HasAttrString ()
#1  0x00007ff669bcdcb7 in PyNs3Object__PythonHelper::GetInstanceTypeId (this=0x7dbba0)
    at debug/bindings/python/ns3module.cc:19465
#2  0x00007ff6693ce4ba in ns3::ObjectBase::ConstructSelf (this=0x7dbba0, attributes=@0x7fff73675f50)
    at ../src/core/object-base.cc:56
#3  0x00007ff6693d26b1 in ns3::Object::Construct (this=0x7dbba0, attributes=@0x7fff73675f50) at ../src/core/object.cc:101
#4  0x00007ff669cc9cbd in ns3::CreateObject<PyNs3Object__PythonHelper> (attributes=@0x7fff73675f50) at debug/ns3/object.h:401
#5  0x00007ff669c926fc in _wrap_create_object_ns3__Object (self=0x7ff66b4f3d40, args=0x7ff66b61b050, kwargs=0x0, 
    return_exception=0x7fff73675fe0) at debug/bindings/python/ns3module.cc:19580
#6  0x00007ff669c9281c in _wrap_PyNs3Object__tp_init (self=0x7ff66b4f3d40, args=0x7ff66b61b050, kwargs=0x0)
    at debug/bindings/python/ns3module.cc:19600
#7  0x000000000045c901 in ?? ()
#8  0x0000000000417e73 in PyObject_Call ()
#9  0x00000000004860ea in PyEval_EvalFrameEx ()
#10 0x000000000048a376 in PyEval_EvalCodeEx ()
#11 0x000000000048a492 in PyEval_EvalCode ()
#12 0x00000000004ac459 in PyRun_InteractiveOneFlags ()
#13 0x00000000004ac664 in PyRun_InteractiveLoopFlags ()
#14 0x00000000004ac76a in PyRun_AnyFileExFlags ()
#15 0x00000000004145ad in Py_Main ()
#16 0x00007ff66a8711c4 in __libc_start_main () from /lib/libc.so.6
#17 0x0000000000413b29 in _start ()
(gdb) quit
Comment 1 Gustavo J. A. M. Carneiro 2008-06-19 17:18:48 UTC
I have a patch to fix this, but it would require making Object::Construct and Object::SetTypeId public.
Comment 2 Gustavo J. A. M. Carneiro 2008-06-19 17:19:34 UTC
Created attachment 177 [details]
patch
Comment 3 Gustavo J. A. M. Carneiro 2008-06-20 09:48:01 UTC
Mathieu please comment on the proposal for making Object::Construct and
Object::SetTypeId public.

Why I need this can be seen from the patch:

+namespace ns3 {
+template <typename T>
+Ptr<T> CreateObjectPython (PyObject *pyobj, const AttributeList &attributes)
+{
+  Ptr<T> p = Ptr<T> (new T (), false);
+  p->set_pyobj (pyobj);  /// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
+  p->SetTypeId (T::GetTypeId ());
+  p->Object::Construct (attributes);
+  return p;  
+}
+
+} // namespace ns3

The function CreateObjectPython is the same as CreateObject, but adds a python specific call before Object::Construct.  This is needed to register the python wrapper with the C++ instance before Object::Construct gets called, because Object::Construct calls a virtual method.  Virtual method wrappers need access to the Python wrapper to check if the wrapper overrides the virtual method in Python, and if so, call the Python implementation.
Comment 4 Mathieu Lacage 2008-06-20 10:55:41 UTC
(In reply to comment #3)
> Mathieu please comment on the proposal for making Object::Construct and
> Object::SetTypeId public.

I hate the idea of making these methods public because they really expose low-level implementation details of that class. Can't you just add a friend method ?

i.e.,
class Object {
private:
friend void PythonCompleteConstruct (Ptr<Object> object, const AttributeList &attributes);
};

Comment 5 Gustavo J. A. M. Carneiro 2008-06-20 11:08:34 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Mathieu please comment on the proposal for making Object::Construct and
> > Object::SetTypeId public.
> 
> I hate the idea of making these methods public because they really expose
> low-level implementation details of that class.

private/protected is but one mechanism for marking APIs for internal use.  Another popular way to accomplish the same goal, maybe you heard of it, is called "documentation".  I hear people say great things about this "documentation" method... :-)

The documentation technique would simply add a notice to the doxygen comment: 

    """Note: this API is meant for integration with external object system implementations, such as language bindings, and should otherwise be considered internal."""

> Can't you just add a friend
> method ?
> 
> i.e.,
> class Object {
> private:
> friend void PythonCompleteConstruct (Ptr<Object> object, const AttributeList
> &attributes);
> };
> 

It's not nice to special case just for Python.  What about the next object system that comes along?  Personally I would prefer to make it public.
Comment 6 Mathieu Lacage 2008-06-20 11:14:55 UTC
(In reply to comment #5)

> > Can't you just add a friend
> > method ?
> > 
> > i.e.,
> > class Object {
> > private:
> > friend void PythonCompleteConstruct (Ptr<Object> object, const AttributeList
> > &attributes);
> > };
> > 
> 
> It's not nice to special case just for Python.  What about the next object
> system that comes along?  Personally I would prefer to make it public.

If/when that happens, we could revisit that issue.
Comment 7 Gustavo J. A. M. Carneiro 2008-06-20 13:04:56 UTC
OK, fixed with Mathieu's friend function suggestion.