correctly handle inter-dictionary dependencies.

when adding them, make the "next" one depend on the one which
loaded it.  Then add the "next" one to the autofree list for the
current dictionary.

When freeing the global context, walk through all of the
dictionaries and free the autoref references first. Then walk through
them again, freeing the externally loaded references.

diff --git a/src/lib/util/dict_fixup.c b/src/lib/util/dict_fixup.c
index 03bff81355f48dc04002d05e42ff4bdf8ac51bb4..408d78f0b338647ddba995a61b0301ab1594842a 100644 (file)
--- a/src/lib/util/dict_fixup.c
+++ b/src/lib/util/dict_fixup.c
@@ -256,12 +256,20 @@ static fr_dict_attr_t const *dict_find_or_load_reference(fr_dict_t **dict_def, c
                p = strchr(name, '.');
                if (p) *p = '\0';
 
-               if (fr_dict_protocol_afrom_file(&dict, name, NULL, filename) < 0) {
+               /*
+                *      Load the new dictionary, and mark it as loaded from our dictionary.
+                */
+               if (fr_dict_protocol_afrom_file(&dict, name, NULL, (*dict_def)->root->name) < 0) {
                        fr_strerror_printf("Unknown protocol '%s' at %s[%d]", name,
                                           fr_cwd_strip(filename), line);
                        return NULL;
                }
 
+               if (!fr_hash_table_insert((*dict_def)->autoref, dict)) {
+                       fr_strerror_const("Failed inserting into internal autoref table");
+                       return NULL;
+               }
+
                /*
                 *      The reference is to the root of the foreign protocol, we're done.
                 */

diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index d4a9eb806cfaaaff33793f1f2bf21abc31722386..76321326a99875d18cb358d8ed674c1398ecbc21 100644 (file)
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -3296,6 +3296,30 @@ bool dict_has_dependents(fr_dict_t *dict)
        return (fr_rb_num_elements(dict->dependents) > 0);
 }
 
+static int dict_autoref_free(fr_dict_t *dict)
+{
+       fr_dict_t **refd_list;
+       unsigned int i;
+
+       if (!dict->autoref) return 0;
+
+       if (fr_hash_table_flatten(NULL, (void ***)&refd_list, dict->autoref) < 0) {
+               fr_strerror_const("failed flattening autoref hash table");
+               return -1;
+       }
+
+       for (i = 0; i < talloc_array_length(refd_list); i++) {
+               if (fr_dict_free(&refd_list[i], dict->root->name) < 0) {
+                       fr_strerror_printf("failed freeing autoloaded protocol %s", refd_list[i]->root->name);
+                       return -1;
+               }
+       }
+
+       TALLOC_FREE(dict->autoref);
+
+       return 0;
+}
+
 static int _dict_free(fr_dict_t *dict)
 {
        /*
@@ -3312,7 +3336,10 @@ static int _dict_free(fr_dict_t *dict)
        }
 
 #ifdef STATIC_ANALYZER
-       if (!dict->root) return -1;
+       if (!dict->root) {
+               fr_strerror_const("dict root is missing");
+               return -1;
+       }
 #endif
 
        if (!fr_cond_assert(!dict->in_protocol_by_name || fr_hash_table_delete(dict->gctx->protocol_by_name, dict))) {
@@ -3338,9 +3365,12 @@ static int _dict_free(fr_dict_t *dict)
                     dep = fr_rb_iter_next_inorder(&iter)) {
                        fr_strerror_printf_push("%s (%u)", dep->dependent, dep->count);
                }
+
                return -1;
        }
 
+       if (dict_autoref_free(dict) < 0) return -1;
+
        /*
         *      Free the hash tables with free functions first
         *      so that the things the hash tables reference
@@ -3348,19 +3378,6 @@ static int _dict_free(fr_dict_t *dict)
         */
        talloc_free(dict->vendors_by_name);
 
-       if (dict->autoref) {
-               fr_dict_t **refd_list;
-               unsigned int i;
-
-               if (fr_hash_table_flatten(NULL, (void ***)&refd_list, dict->autoref) < 0) return -1;
-
-               for (i = 0; i < talloc_array_length(refd_list); i++) {
-                    if (fr_dict_free(&refd_list[i], dict->root->name) < 0) return -1;
-               }
-
-               talloc_free(refd_list);
-       }
-
        /*
         *      Decrease the reference count on the validation
         *      library we loaded.
@@ -3893,13 +3910,29 @@ static int _dict_global_free(fr_dict_gctx_t *gctx)
                if (talloc_free(gctx->internal) < 0) still_loaded = true;
        }
 
+       /*
+        *      Free up autorefs first, which will free up inter-dictionary dependencies.
+        */
+       for (dict = fr_hash_table_iter_init(gctx->protocol_by_name, &iter);
+            dict;
+            dict = fr_hash_table_iter_next(gctx->protocol_by_name, &iter)) {
+               (void)talloc_get_type_abort(dict, fr_dict_t);
+
+               if (dict_autoref_free(dict) < 0) return -1;
+       }
+
        for (dict = fr_hash_table_iter_init(gctx->protocol_by_name, &iter);
             dict;
             dict = fr_hash_table_iter_next(gctx->protocol_by_name, &iter)) {
                (void)talloc_get_type_abort(dict, fr_dict_t);
                dict_dependent_remove(dict, "global");                  /* remove our dependency */
 
-               if (talloc_free(dict) < 0) still_loaded = true;
+               if (talloc_free(dict) < 0) {
+#ifndef NDEBUG
+                       FR_FAULT_LOG("gctx failed to free dictionary %s - %s", dict->root->name, fr_strerror());
+#endif
+                       still_loaded = true;
+               }
        }
 
        if (still_loaded) {