Please refrain from doing so, the assumption #1 this patch is based on is false. First, OBJ_CONSTRUCT can be run to construct a specific type of object in a preallocated memory region (not only on the stack or heap). In fact, it is the only way we can dynamically initialize an object in a memory allocated outside the OBJ_NEW system. Second, OBJ_CONSTRUCT is not necessarily matched with an OBJ_DESTRUCT, it work just fine with OBJ_RELEASE. In fact I use these feature in several places.
An example will be a memory region without a predefined size, that I manipulate as opal_list_item_t. This fragment gets allocated when it's size is know, then gets OBJ_CONSTRUCT'ed and then used. The reference count is playing its role, when nobody is using the object anymore, it will be automatically released. With the change you propose such usage will be prohibited. The feature you are looking for, the one that might have shorten Ralph's debugging time, is already in the opal_object_t. One should use the cls_init_file_name and cls_init_lineno fields to see where the object was first initialized as these fields are set either by the OBJ_NEW or by the OBJ_CONSTRUCT call. George. PS: The second patch (ref count == 1 in OBJ_DESTRUCT) is trivial but reasonable. On Mar 7, 2013, at 22:10 , Jeff Squyres (jsquyres) <jsquy...@cisco.com> wrote: > WHAT: Simple patch that will fail an assert() if you OBJ_CONSTRUCT an object > and its ref count goes to 0 (in debug builds only). > > WHY: To catch bugs. > > WHERE: opal/class/opal_class.h > > WHEN: Timeout of 1 week -- COB, Thurs, 14 Mar, 2013 > > MORE DETAIL: > > On the call on Tuesday, we talked about some ideas for catching bugs with > respect to object reference counting. After the call, Brian, Ralph, and I > came up with two useful asserts to help catch bugs (in debug builds only): > > 1. If you OBJ_CONSTRUCT an object, its ref count should never go to zero. > 2. When a object is destroyed, its refcount should be exactly 1. > > This RFC is for #1. The attached patch doesn't seem to cause any problems > (and we didn't expect it to). But it's a good addition to the other > asserts() that are already in the object code already. > > As for #2, Ralph has previously found bugs in the ORTE layer that would have > been much easier to track down if #2 were in place. I'll send an RFC for #2 > when I have managed to fix all the problems that it has found in the OMPI > layer... :-) > > -- > Jeff Squyres > jsquy...@cisco.com > For corporate legal information go to: > http://www.cisco.com/web/about/doing_business/legal/cri/ > > Index: opal/class/opal_object.h > =================================================================== > --- opal/class/opal_object.h (revision 28147) > +++ opal/class/opal_object.h (working copy) > @@ -169,7 +169,7 @@ > * @param NAME Name of the class to initialize > */ > #if OPAL_ENABLE_DEBUG > -#define OPAL_OBJ_STATIC_INIT(BASE_CLASS) { OPAL_OBJ_MAGIC_ID, > OBJ_CLASS(BASE_CLASS), 1, __FILE__, __LINE__ } > +#define OPAL_OBJ_STATIC_INIT(BASE_CLASS) { OPAL_OBJ_MAGIC_ID, 1, > OBJ_CLASS(BASE_CLASS), 1, __FILE__, __LINE__ } > #else > #define OPAL_OBJ_STATIC_INIT(BASE_CLASS) { OBJ_CLASS(BASE_CLASS), 1 } > #endif > @@ -184,6 +184,10 @@ > /** Magic ID -- want this to be the very first item in the > struct's memory */ > uint64_t obj_magic_id; > + /* flag whether this was initialized using construct > + * versus obj_new > + */ > + bool constructed; > #endif > opal_class_t *obj_class; /**< class descriptor */ > volatile int32_t obj_reference_count; /**< reference count */ > @@ -252,6 +256,7 @@ > object->obj_magic_id = OPAL_OBJ_MAGIC_ID; > object->cls_init_file_name = file; > object->cls_init_lineno = line; > + object->constructed = false; > return object; > } > #define OBJ_NEW(type) \ > @@ -313,6 +318,8 @@ > assert(NULL != ((opal_object_t *) (object))->obj_class); \ > assert(OPAL_OBJ_MAGIC_ID == ((opal_object_t *) > (object))->obj_magic_id); \ > if (0 == opal_obj_update((opal_object_t *) (object), -1)) { \ > + /* constructed objects are not allowed to reach 0 */ \ > + assert(!(((opal_object_t *) (object))->constructed)); \ > OBJ_SET_MAGIC_ID((object), 0); \ > opal_obj_run_destructors((opal_object_t *) (object)); \ > OBJ_REMEMBER_FILE_AND_LINENO( object, __FILE__, __LINE__ ); \ > @@ -344,6 +351,7 @@ > OBJ_CONSTRUCT_INTERNAL((object), OBJ_CLASS(type)); \ > } while (0) > > +#if OPAL_ENABLE_DEBUG > #define OBJ_CONSTRUCT_INTERNAL(object, type) \ > do { \ > OBJ_SET_MAGIC_ID((object), OPAL_OBJ_MAGIC_ID); \ > @@ -352,11 +360,24 @@ > } \ > ((opal_object_t *) (object))->obj_class = (type); \ > ((opal_object_t *) (object))->obj_reference_count = 1; \ > + ((opal_object_t *) (object))->constructed = true; \ > opal_obj_run_constructors((opal_object_t *) (object)); \ > OBJ_REMEMBER_FILE_AND_LINENO( object, __FILE__, __LINE__ ); \ > } while (0) > +#else > +#define OBJ_CONSTRUCT_INTERNAL(object, type) \ > +do { \ > + OBJ_SET_MAGIC_ID((object), OPAL_OBJ_MAGIC_ID); \ > + if (0 == (type)->cls_initialized) { \ > + opal_class_initialize((type)); \ > + } \ > + ((opal_object_t *) (object))->obj_class = (type); \ > + ((opal_object_t *) (object))->obj_reference_count = 1; \ > + opal_obj_run_constructors((opal_object_t *) (object)); \ > + OBJ_REMEMBER_FILE_AND_LINENO( object, __FILE__, __LINE__ ); \ > +} while (0) > +#endif > > - > /** > * Destruct (finalize) an object that is not dynamically allocated. > * > > > _______________________________________________ > devel mailing list > de...@open-mpi.org > http://www.open-mpi.org/mailman/listinfo.cgi/devel
