Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: [OMPI devel] RFC: assert() to ensure OBJ_CONSTRUCT'ed objects don't get destroyed
From: Jeff Squyres (jsquyres) (jsquyres_at_[hidden])
Date: 2013-03-07 16:10:35


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
jsquyres_at_[hidden]
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.
  *