On Jul 18, 2014, at 8:25 AM, Gilles Gouaillardet <gilles.gouaillar...@gmail.com> wrote:
> +1 for the overall idea ! > > On Fri, Jul 18, 2014 at 10:17 PM, Ralph Castain <r...@open-mpi.org> wrote: >> * add an OBJ_CLASS_DEREGISTER and require that all instantiations be matched >> by deregister at close of the framework/component that instanced it. Of >> course, that requires that we protect the class system against someone >> releasing/deconstructing an object after the class was deregistered since we >> don't know who might be using that class outside of where it was created. >> > > my understanding is that in theory, we already have an issue and fortunatly, > we do not hit it : > let's consider a framework/component that instanciate a class > (OBJ_CLASS_INSTANCE) *with a destructor*, allocate an object of this class > (OBJ_NEW) and expects "someone else" will free it (OBJ_RELEASE) > if this framework/component ends up in a dynamic library that is dlclose'd > when the framework/component is no more used, then OBJ_RELEASE will try to > call the destructor which is no more accessible (since the lib was dlclose'd) FWIW: Intel has hit that exact scenario in our testing and got a glorious segv out of it. We now have an assert for NULL in the base object macro's to warn us to fix it (which I can provide for review if we want to include it in our repo). > > i could not experience such a scenario yet, and of course, this does not mean > there is no problem. i experienced a "kind of" similar situation described in > http://www.open-mpi.org/community/lists/devel/2014/06/14937.php > > back to OBJ_CLASS_DEREGISTER, what about an OBJ_CLASS_REGISTER in order to > make this symmetric and easier to debug ? > > currently, OBJ_CLASS_REGISTER is "implied" the first time an object of a > given class is allocated. from opal_obj_new : > if (0 == cls->cls_initialized) opal_class_initialize(cls); > > that could be replaced by an error if 0 == cls->cls_initialized > and OBJ_CLASS_REGISTER would simply call opal_class_initialize It would make sense, though I guess I always thought that was part of what happened in OBJ_CLASS_INSTANCE - guess I was wrong. My thinking was that DEREGISTER would be the counter to INSTANCE, and I do want to keep this from getting even more clunky - so maybe renaming INSTANCE to be REGISTER and completing the initialization inside it would be the way to go. Or renaming DEREGISTER to something more obviously the counter to INSTANCE? > > of course, this change could be implemented only when compiled > with OPAL_ENABLE_DEBUG > > Cheers, > > Gilles > _______________________________________________ > devel mailing list > de...@open-mpi.org > Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel > Link to this post: > http://www.open-mpi.org/community/lists/devel/2014/07/15196.php