Bugzilla – Bug 1094
Object::GetObject fails when ns-3 modules are dlopen'ed
Last modified: 2011-05-18 13:38:05 UTC
I was debugging recent Python bindings failures, and the issue I got was that GetObject was failing. In DoGetObject (object.cc) I see that it is finding the right object. Then the value is returned to GetObject (object.h), which does a dynamic_cast. The dynamic_cast fails, and so NULL pointer is returned. I have heard tales of dynamic_cast failing across dlopen'ed libraries before, so I am not surprised. More modern systems seem to not exhibit this problem, which is good, but for greater portability I propose the following patch, which replaces dynamic_cast with reinterpret_cast: 1. dynamic_cast does runtime type checking, with a cost, so I have to wonder why we are calling it anyway? 2. DoGetObject already does the type checking, using ns-3's proprietary RTTI, so by calling dynamic_cast we are just wasting time with the C++ RTTI for no benefit (and this bug report as drawback). diff -r 53f1bfb3e91c src/core/model/object.h --- a/src/core/model/object.h Mon Apr 04 19:44:35 2011 +0100 +++ b/src/core/model/object.h Mon Apr 04 20:05:44 2011 -0400 @@ -396,7 +396,7 @@ Object::GetObject () const Ptr<Object> found = DoGetObject (T::GetTypeId ()); if (found != 0) { - return Ptr<T> (dynamic_cast<T *> (PeekPointer (found))); + return Ptr<T> (reinterpret_cast<T *> (PeekPointer (found))); } return 0; } @@ -408,7 +408,7 @@ Object::GetObject (TypeId tid) const Ptr<Object> found = DoGetObject (tid); if (found != 0) { - return Ptr<T> (dynamic_cast<T *> (PeekPointer (found))); + return Ptr<T> (reinterpret_cast<T *> (PeekPointer (found))); } return 0; }
I checked in a version of your patch that uses static_cast instead of reinterpret_cast on the grounds that it cannot harm in any way. But, really, I am worried that if what you say is true, grep dynamic_cast in src/core/model shows a lot of serious potential problems. Note: initial bug report waf for ubuntu 0804 changeset: 00c8ea8e9e40
There is nothing I can do to prevent Python from dlopen'ing the modules. But things seem to work fine with the patch, at least utils/python-unit-tests.py. We'll see if the buildbot reports more problems...
(In reply to comment #1) > I checked in a version of your patch that uses static_cast instead of > reinterpret_cast on the grounds that it cannot harm in any way. But, really, I > am worried that if what you say is true, grep dynamic_cast in src/core/model > shows a lot of serious potential problems. > > Note: initial bug report waf for ubuntu 0804 > I observed this also on ubuntu 9.10 (ns-ubuntu-karmic machine in the UW regression testbed). Before the patch, I saw all six python examples failing. After applying the patch for this bug (changeset 00c8ea8e9e40), I see these test results: CRASH: Example examples/routing/simple-routing-ping6.py PASS: Example examples/wireless/wifi-ap.py CRASH: Example examples/wireless/mixed-wireless.py PASS: Example examples/tutorial/first.py CRASH: Example src/bridge/examples/csma-bridge.py CRASH: Example src/flow-monitor/examples/wifi-olsr-flowmon.py
I think that dynamic_cast fails when the original type and downcasted type are in different dlopen'ed libraries. C++ implementation bugs... We can: 1. Get rid of all cross-module dynamic_casts (dynamic casts between types in the same module are ok). Perhaps replacing dynamic_cast with Object::GetObject, where possible... 2. Link python modules to all ns-3 module libraries, not just the one that they need wrap.
(In reply to comment #4) > I think that dynamic_cast fails when the original type and downcasted type are > in different dlopen'ed libraries. C++ implementation bugs... > > We can: > > 1. Get rid of all cross-module dynamic_casts (dynamic casts between types in > the same module are ok). Perhaps replacing dynamic_cast with > Object::GetObject, where possible... > > 2. Link python modules to all ns-3 module libraries, not just the one that they > need wrap. This bug is fixed?