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. *
