Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] RFC: Add an __attribute__((destructor)) function to opal
From: Ralph Castain (rhc_at_[hidden])
Date: 2014-07-18 11:40:15


On Jul 18, 2014, at 8:25 AM, Gilles Gouaillardet <gilles.gouaillardet_at_[hidden]> wrote:

> +1 for the overall idea !
>
> On Fri, Jul 18, 2014 at 10:17 PM, Ralph Castain <rhc_at_[hidden]> 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
> devel_at_[hidden]
> 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