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

Reply via email to