sde_lib: Added reference counts for proper unregistering/deleting of …

…groups.

Counters can belong in groups, even multiple groups, and groups can recursively
belong in larger groups. This means that a counter (or group) cannot be
unregistered and freed without keep track of which groups it belong to.
Now each counter has a reference counter "ref_count" which is incremented
when it's added in a group and decremented when the counter is unregistered,
or when a parent group is unregistered.

src/sde_lib/sde_lib.c

@@ -211,36 +211,30 @@ papi_sde_enable( papi_handle_t handle ){
 int
 papi_sde_shutdown( papi_handle_t handle ){
     papisde_library_desc_t *lib_handle, *tmp_lib, *next_lib, *prev_lib;
-    papisde_list_entry_t *list_head, *curr;
-    int i, ret_val=SDE_OK;
+    int i;
 
     lib_handle = (papisde_library_desc_t *) handle;
     papisde_control_t *gctl = _papisde_global_control;
     if( (NULL==lib_handle) || lib_handle->disabled || (NULL==gctl) || gctl->disabled)
         return SDE_OK;
 
+    SDEDBG("papi_sde_shutdown(): for library '%s'.\n", lib_handle->libraryName);
+
     sde_lock();
-    for(i=0; i<PAPISDE_HT_SIZE; i++){
-        list_head = &(lib_handle->lib_counters[i]);
-        if( NULL == list_head )
-            continue;
 
-        if(NULL != list_head->item){
-            sdei_free_counter(list_head->item);
-        }
+    sde_counter_t *all_lib_counters;
+    int item_cnt = ht_to_array(lib_handle->lib_counters, &all_lib_counters);
 
-        for(curr = list_head->next; NULL != curr; curr=list_head->next){
-            if(NULL == curr->item){ // This can only legally happen for the head of the list.
-                SDE_ERROR("papi_sde_shutdown(): the counter hash table is clobbered.");
-                ret_val = SDE_EINVAL;
-                goto fn_exit;
-            }
-            sdei_free_counter(curr->item);
-            list_head->next = curr->next;
-            free(curr);
-        }
+    for(i=0; i<item_cnt; i++){
+        char *cntr_name = all_lib_counters[i].name;
+        sdei_delete_counter(lib_handle, cntr_name);
     }
 
+    // We don't need the serialized array any more. Besides, the pointers inside
+    // its elements have _not_ been copied, so they are junk by now, since we
+    // deleted the counters.
+    free(all_lib_counters);
+
     // Keep the `gctl` struct consistent
     // 1. If the lib head is this one, just set to next (could be NULL)
     // 2. Otherwise, find the prev_lib and set prev_lib->next = next_lib
@@ -263,9 +257,8 @@ papi_sde_shutdown( papi_handle_t handle ){
     free(lib_handle->libraryName);
     free(lib_handle);
 
-fn_exit:
     sde_unlock();
-    return ret_val;
+    return SDE_OK;
 }
 
 
@@ -353,7 +346,7 @@ papi_sde_unregister_counter( papi_handle_t handle, const char *event_name)
     char *full_event_name;
     int ret_val;
 
-    SDEDBG("Preparing to unregister counter\n");
+    SDEDBG("Preparing to unregister counter: '%s'.\n",event_name);
 
     lib_handle = (papisde_library_desc_t *) handle;
     papisde_control_t *gctl = _papisde_global_control;
@@ -525,7 +518,6 @@ papi_sde_add_counter_to_group(papi_handle_t handle, const char *event_name, cons
         (void)ht_insert(gctl->all_reg_counters, ht_hash_id(cntr_group_uniq_id), tmp_group);
 
     }else{
-        // should the following branch ever be true? Why do we already have a group registered if it's empty?
         if( NULL == tmp_group->u.cntr_group.group_head ){
             SDE_ERROR("papi_sde_add_counter_to_group(): Found an empty counter group: '%s'. This might indicate that a cleanup routine is not doing its job.", group_name);
         }
@@ -544,6 +536,9 @@ papi_sde_add_counter_to_group(papi_handle_t handle, const char *event_name, cons
     new_head->item = tmp_item;
     new_head->next = tmp_group->u.cntr_group.group_head;
     tmp_group->u.cntr_group.group_head = new_head;
+    if( SDE_OK != sdei_inc_ref_count(tmp_item) ){
+        SDE_ERROR("papi_sde_add_counter_to_group(): Error while adding counter '%s' to counter group: '%s'.", tmp_item->name, group_name);
+    }
 
     free(full_group_name);
     ret_val = SDE_OK;

src/sde_lib/sde_lib.h

@@ -41,6 +41,7 @@
 #define SDE_ENOMEM     -2     /**< Insufficient memory */
 #define SDE_ECMP       -4     /**< Not supported by component */
 #define SDE_ENOEVNT    -7     /**< Event does not exist */
+#define SDE_EMISC      -14    /**< Unknown error code */
 
 #define register_fp_counter register_counter_cb
 #define papi_sde_register_fp_counter papi_sde_register_counter_cb

src/sde_lib/sde_lib_internal.h

@@ -139,6 +139,7 @@ struct sde_counter_s {
    int overflow;
    int cntr_type;
    int cntr_mode;
+   int ref_count;
    papisde_library_desc_t *which_lib;
 };
 
@@ -187,7 +188,8 @@ struct papisde_control_s {
 
 int sdei_setup_counter_internals( papi_handle_t handle, const char *event_name, int cntr_mode, int cntr_type, enum CNTR_CLASS cntr_class, cntr_class_specific_t cntr_union );
 int sdei_delete_counter(papisde_library_desc_t* lib_handle, const char *name);
-int sdei_free_counter(sde_counter_t *counter);
+int sdei_inc_ref_count(sde_counter_t *counter);
+int sdei_free_counter_resources(sde_counter_t *counter);
 int sdei_read_counter_group( sde_counter_t *counter, long long int *rslt_ptr );
 void sdei_counting_set_to_list( void *cset_handle, cset_list_object_t **list_head );
 int sdei_read_and_update_data_value( sde_counter_t *counter, long long int previous_value, long long int *rslt_ptr );

src/sde_lib/sde_lib_misc.c

@@ -232,82 +232,160 @@ int sdei_setup_counter_internals( papi_handle_t handle, const char *event_name,
     return ret_val;
 }
 
-int sdei_delete_counter(papisde_library_desc_t* lib_handle, const char* name) {
+int sdei_inc_ref_count(sde_counter_t *counter){
+    papisde_list_entry_t *curr;
+    if( NULL == counter )
+        return SDE_OK;
+
+    // If the counter is a group, recursivelly increment the ref_count of all its children.
+    if(CNTR_CLASS_GROUP == counter->cntr_class){
+        curr = counter->u.cntr_group.group_head;
+        do{
+            sde_counter_t *tmp_cntr = curr->item;
+            // recursively increment the ref_count of all the elements in the group.
+            int ret_val = sdei_inc_ref_count(tmp_cntr);
+            if( SDE_OK != ret_val )
+                return ret_val;
+            curr = curr->next;
+        }while(NULL != curr);
+    }
 
+    // Increment the ref_count of the counter itself, INCLUDING the case where the counter is a group.
+    (counter->ref_count)++;
+
+    return SDE_OK;
+}
+
+int sdei_delete_counter(papisde_library_desc_t* lib_handle, const char* name) {
     sde_counter_t *tmp_item;
     papisde_control_t *gctl;
     uint32_t item_uniq_id;
+    int ret_val = SDE_OK;
 
     gctl = sdei_get_global_struct();
 
     // Look for the counter entry in the hash-table of the library
     tmp_item = ht_lookup_by_name(lib_handle->lib_counters, name);
-    if( NULL == tmp_item )
-        return 1;
-
-    item_uniq_id = tmp_item->glb_uniq_id;
-
-    // Delete the entry from the library hash-table (which hashes by name)
-    tmp_item = ht_delete(lib_handle->lib_counters, ht_hash_name(name), item_uniq_id);
     if( NULL == tmp_item ){
-        return 1;
+        ret_val = SDE_EINVAL;
+        goto fn_exit;
     }
 
-    // Delete the entry from the global hash-table (which hashes by id) and free the memory
-    // occupied by the counter (not the hash-table entry 'papisde_list_entry_t', the 'sde_counter_t')
-    tmp_item = ht_delete(gctl->all_reg_counters, ht_hash_id(item_uniq_id), item_uniq_id);
-    if( NULL == tmp_item ){
-        return 1;
+    if( CNTR_CLASS_GROUP == tmp_item->cntr_class ){
+        papisde_list_entry_t *curr, *prev;
+        // If we are dealing with a goup, then we need to recurse down all its children and
+        // delete them (this might mean free them, or just decrement their ref_count).
+        curr = tmp_item->u.cntr_group.group_head;
+        prev = curr;
+        while(NULL != curr){
+            int counter_is_dead = 0;
+            sde_counter_t *tmp_cntr = curr->item;
+            if( NULL == tmp_cntr ){
+                ret_val = SDE_EMISC;
+                goto fn_exit;
+            }
+
+            // If this counter is going to be freed, we need to remove it from this group.
+            if( 0 == tmp_cntr->ref_count )
+                counter_is_dead = 1;
+
+            // recursively delete all the elements of the group.
+            int ret_val = sdei_delete_counter(lib_handle, tmp_cntr->name);
+            if( SDE_OK != ret_val )
+                goto fn_exit;
+
+            if( counter_is_dead ){
+                if( curr == tmp_item->u.cntr_group.group_head ){
+                    // if we were removing with the head, change the head, we can't free() it.
+                    tmp_item->u.cntr_group.group_head = curr->next;
+                    prev = curr->next;
+                    curr = curr->next;
+                }else{
+                    // if we are removing an element, first bridge the previous to the next.
+                    prev->next = curr->next;
+                    free(curr);
+                    curr = prev->next;
+                }
+            }else{
+                // if we are not removing anything, just move the pointers.
+                prev = curr;
+                curr = curr->next;
+            }
+        }
     }
 
-    // We free the counter only once, although it is in two hash-tables,
-    // because it is the same structure that is pointed to by both hash-tables.
-    sdei_free_counter(tmp_item);
+    item_uniq_id = tmp_item->glb_uniq_id;
+
+    // If the reference count is not zero, then we don't remove it from the hash tables
+    if( 0 == tmp_item->ref_count ){
+        // Delete the entry from the library hash-table (which hashes by name)
+        tmp_item = ht_delete(lib_handle->lib_counters, ht_hash_name(name), item_uniq_id);
+        if( NULL == tmp_item ){
+            ret_val = SDE_EMISC;
+            goto fn_exit;
+        }
 
-    // Decrement the number of live events.
-    gctl->num_live_events--;
+        // Delete the entry from the global hash-table (which hashes by id) and free the memory
+        // occupied by the counter (not the hash-table entry 'papisde_list_entry_t', the 'sde_counter_t')
+        tmp_item = ht_delete(gctl->all_reg_counters, ht_hash_id(item_uniq_id), item_uniq_id);
+        if( NULL == tmp_item ){
+            ret_val = SDE_EMISC;
+            goto fn_exit;
+        }
+
+        // We free the counter only once, although it is in two hash-tables,
+        // because it is the same structure that is pointed to by both hash-tables.
+        // This function will check internally if the reference count is zero, because it
+        // needs to recursivelly access all the children of groups.
+        sdei_free_counter_resources(tmp_item);
 
-    return 0;
+        // Decrement the number of live events.
+        (gctl->num_live_events)--;
+    }else{
+        (tmp_item->ref_count)--;
+    }
+
+fn_exit:
+    return ret_val;
 }
 
-int sdei_free_counter(sde_counter_t *counter){
+int sdei_free_counter_resources(sde_counter_t *counter){
     int i, ret_val = SDE_OK;
-    papisde_list_entry_t *curr;
 
     if( NULL == counter )
         return SDE_OK;
 
-    free(counter->name);
-    free(counter->description);
+    if( counter->ref_count < 0 ){
+        SDE_ERROR("sdei_free_counter_resources(): Counter has a negative ref_count.\n");
+        return SDE_EINVAL;
+    }
 
-    switch(counter->cntr_class){
-        case CNTR_CLASS_CREATED:
-            free(counter->u.cntr_basic.data);
-            break;
-        case CNTR_CLASS_RECORDER:
-            free(counter->u.cntr_recorder.data->sorted_buffer);
-            for(i=0; i<EXP_CONTAINER_ENTRIES; i++){
-                free(counter->u.cntr_recorder.data->ptr_array[i]);
-            }
-            free(counter->u.cntr_recorder.data);
-            break;
-        case CNTR_CLASS_CSET:
-            ret_val = cset_delete(counter->u.cntr_cset.data);
-            break;
-        case CNTR_CLASS_GROUP:
-            curr = counter->u.cntr_group.group_head;
-            do{
-                sde_counter_t *tmp_cntr = curr->item;
-                // recursively free all the elements of the group.
-                ret_val = sdei_free_counter(tmp_cntr);
-                if( SDE_OK != ret_val )
-                    return ret_val;
-                curr = curr->next;
-            }while(NULL != curr);
-            break;
+    if( 0 == counter->ref_count ){
+        switch(counter->cntr_class){
+            case CNTR_CLASS_CREATED:
+                SDEDBG(" + Freeing Created Counter Data.\n");
+                free(counter->u.cntr_basic.data);
+                break;
+            case CNTR_CLASS_RECORDER:
+                SDEDBG(" + Freeing Recorder Data.\n");
+                free(counter->u.cntr_recorder.data->sorted_buffer);
+                for(i=0; i<EXP_CONTAINER_ENTRIES; i++){
+                    free(counter->u.cntr_recorder.data->ptr_array[i]);
+                }
+                free(counter->u.cntr_recorder.data);
+                break;
+            case CNTR_CLASS_CSET:
+                SDEDBG(" + Freeing CountingSet Data.\n");
+                ret_val = cset_delete(counter->u.cntr_cset.data);
+                break;
+        }
+
+        SDEDBG(" -> Freeing Counter '%s'.\n",counter->name);
+        free(counter->name);
+        free(counter->description);
+        free(counter);
     }
 
-    free(counter);
     return ret_val;
 }