ly_common UPDATE pattern caching and freeing

Author: Roytak

src/ly_common.c

@@ -141,20 +141,6 @@ ly_ctx_ht_pattern_equal_cb(void *val1_p, void *val2_p, ly_bool UNUSED(mod), void
     return !strcmp(val1->pattern, val2->pattern);
 }
 
-/**
- * @brief Callback for freeing a pattern record.
- */
-static void
-ly_ctx_ht_pattern_free_cb(void *val_p)
-{
-    struct ly_pattern_ht_rec *val = val_p;
-
-    if (val->pcode) {
-        /* free the pcode */
-        pcre2_code_free(val->pcode);
-    }
-}
-
 /**
  * @brief Remove private context data from the sized array and free its contents.
  *
@@ -304,8 +290,11 @@ ly_ctx_shared_data_remove_and_free(struct ly_ctx_shared_data *shared_data)
         return;
     }
 
-    /* free all the cached pattern pcodes */
-    lyht_free(shared_data->pattern_ht, ly_ctx_ht_pattern_free_cb);
+    /* all the patterns must have been removed already,
+     * either while free compiled modules (standard behavior)
+     * or when assigning a parent to a context, it's shared data will be used (schema mount) */
+    assert(shared_data->pattern_ht->used == 0);
+    lyht_free(shared_data->pattern_ht, NULL);
 
     /* free rest of the members */
     lydict_clean(shared_data->data_dict);
@@ -544,6 +533,16 @@ ly_ctx_data_del(const struct ly_ctx *ctx)
     pthread_rwlock_unlock(&ly_ctx_data_rwlock);
 }
 
+void
+ly_ctx_ht_pattern_rec_free(struct ly_pattern_ht_rec *rec)
+{
+    if (!rec) {
+        return;
+    }
+
+    pcre2_code_free(rec->pcode);
+}
+
 LY_ERR
 ly_ctx_shared_data_pattern_get(const struct ly_ctx *ctx, const char *pattern, const pcre2_code **pcode)
 {
@@ -559,39 +558,36 @@ ly_ctx_shared_data_pattern_get(const struct ly_ctx *ctx, const char *pattern, co
         *pcode = NULL;
     }
 
+    /* get the context shared data */
     ctx_data = ly_ctx_shared_data_get(ctx);
     LY_CHECK_RET(!ctx_data, LY_EINT);
 
     /* try to find the pattern code in the pattern ht */
     hash = lyht_hash(pattern, strlen(pattern));
     rec.pattern = pattern;
     if (!lyht_find(ctx_data->pattern_ht, &rec, hash, (void **)&found_rec)) {
-        /* found it, return it */
+        /* pcode cached */
         if (pcode) {
             *pcode = found_rec->pcode;
         }
         goto cleanup;
     }
 
-    /* didnt find it, need to compile it */
+    /* didnt find it, either it's the first time or using printed context (which compiles the pcodes on the fly) */
+    assert(!pcode || ly_ctx_is_printed(ctx));
     LY_CHECK_GOTO(rc = lys_compile_type_pattern_check(ctx, pattern, &pcode_tmp), cleanup);
 
     /* store the compiled pattern code in the hash table */
-    hash = lyht_hash(pattern, strlen(pattern));
-    rec.pattern = pattern;
     rec.pcode = pcode_tmp;
     LY_CHECK_GOTO(rc = lyht_insert_no_check(ctx_data->pattern_ht, &rec, hash, NULL), cleanup);
 
     if (pcode) {
         *pcode = pcode_tmp;
-        pcode_tmp = NULL;
     }
+    pcode_tmp = NULL;
 
 cleanup:
-    if (rc) {
-        /* only free the pcode if we failed, because it belongs to the hash table */
-        pcre2_code_free(pcode_tmp);
-    }
+    pcre2_code_free(pcode_tmp);
     return rc;
 }
 
@@ -613,7 +609,8 @@ ly_ctx_shared_data_pattern_del(const struct ly_ctx *ctx, const char *pattern)
     rec.pattern = pattern;
 
     if (lyht_find(ctx_data->pattern_ht, &rec, hash, (void **)&found_rec)) {
-        /* pattern code not cached yet */
+        /* pattern code not cached, this may happen when using printed context,
+         * because then the pcodes are obtained on demand */
         return;
     }
 

src/ly_common.h

@@ -326,6 +326,15 @@ int LY_VCODE_INSTREXP_len(const char *str);
  * Context
  *****************************************************************************/
 
+/**
+ * @brief Internal pattern hash table record.
+ */
+struct ly_pattern_ht_rec {
+    const char *pattern;    /**< Pattern expression, used both as key to hash and value to search for.
+                              * Not stored in a dictionary, but a direct reference to ::lysc_pattern.expr. */
+    pcre2_code *pcode;      /**< Compiled PCRE2 pattern code. */
+};
+
 /**
   * @brief Private run-time context data.
   *
@@ -493,6 +502,13 @@ LY_ERR ly_ctx_shared_data_pattern_get(const struct ly_ctx *ctx, const char *patt
  */
 void ly_ctx_shared_data_pattern_del(const struct ly_ctx *ctx, const char *pattern);
 
+/**
+ * @brief Free members of a pattern record stored in the context hash table.
+ *
+ * @param[in] rec Pattern records whose members to free.
+ */
+void ly_ctx_ht_pattern_rec_free(struct ly_pattern_ht_rec *rec);
+
 /******************************************************************************
  * Dictionary
  *****************************************************************************/

src/plugins_exts.c

@@ -763,6 +763,10 @@ LIBYANG_API_DEF LY_ERR
 lyplg_ext_set_parent_ctx(struct ly_ctx *ctx, const struct ly_ctx *parent_ctx)
 {
     const struct ly_ctx *c;
+    struct ly_ctx_shared_data *ctx_data;
+    struct ly_ht_rec *rec;
+    uint32_t hlist_idx;
+    uint32_t rec_idx;
 
     LY_CHECK_ARG_RET(ctx, ctx, LY_EINVAL);
 
@@ -787,7 +791,16 @@ lyplg_ext_set_parent_ctx(struct ly_ctx *ctx, const struct ly_ctx *parent_ctx)
              * contexts, there is no ext callback for freeing the compiled extension data with the contexts) */
             ly_ctx_destroy(ctx);
         } else {
-            /* remove its shared and private data */
+            /* remove its shared and private data, this is an exception as we need to free compiled patterns
+             * manually, since we are not destroying the whole context, we will just be using the parent's ctx data instead */
+            ctx_data = ly_ctx_shared_data_get(ctx);
+            LYHT_ITER_ALL_RECS(ctx_data->pattern_ht, hlist_idx, rec_idx, rec) {
+                ly_ctx_ht_pattern_rec_free((struct ly_pattern_ht_rec *)&rec->val);
+            }
+
+            /* we have removed all patterns (so it is empty), we can not free the ht here though, to avoid
+             * double free, but just trick it to look empty */
+            ctx_data->pattern_ht->used = 0;
             ly_ctx_data_del(ctx);
         }
     } else if (!parent_ctx) {

src/schema_compile_node.c

@@ -1385,10 +1385,7 @@ lys_compile_type_patterns(struct lysc_ctx *ctx, const struct lysp_restr *pattern
     LY_ARRAY_FOR(patterns_p, u) {
         LY_ARRAY_NEW_RET(ctx->ctx, (*patterns), pattern, LY_EMEM);
         *pattern = calloc(1, sizeof **pattern);
-        ++(*pattern)->refcount;
-
-        /* compile and insert the pattern into the context hash table, if it wasnt already compiled */
-        LY_CHECK_GOTO(ret = ly_ctx_shared_data_pattern_get(ctx->ctx, &patterns_p[u].arg.str[1], NULL), done);
+        (*pattern)->refcount = 1;
 
         if (patterns_p[u].arg.str[0] == LYSP_RESTR_PATTERN_NACK) {
             (*pattern)->inverted = 1;
@@ -1399,6 +1396,12 @@ lys_compile_type_patterns(struct lysc_ctx *ctx, const struct lysp_restr *pattern
         DUP_STRING_GOTO(ctx->ctx, patterns_p[u].dsc, (*pattern)->dsc, ret, done);
         DUP_STRING_GOTO(ctx->ctx, patterns_p[u].ref, (*pattern)->ref, ret, done);
         COMPILE_EXTS_GOTO(ctx, patterns_p[u].exts, (*pattern)->exts, (*pattern), ret, done);
+
+        /* compile the pattern, if it wasnt already compiled (e.g. same expression but different module)
+         * we do this now to verify the pattern right away while compiling the type
+         * and so we can store it in the cache so we dont have to recompile it again later
+         */
+        LY_CHECK_GOTO(ret = ly_ctx_shared_data_pattern_get(ctx->ctx, (*pattern)->expr, NULL), done);
     }
 
 done:

src/tree_schema_free.c

@@ -825,6 +825,10 @@ lysc_pattern_free(const struct ly_ctx *ctx, struct lysc_pattern **pattern)
     if (--(*pattern)->refcount) {
         return;
     }
+
+    /* free the shared pattern first, before removing from the dictionary */
+    ly_ctx_shared_data_pattern_del(ctx, (*pattern)->expr);
+
     lysdict_remove(ctx, (*pattern)->expr);
     lysdict_remove(ctx, (*pattern)->eapptag);
     lysdict_remove(ctx, (*pattern)->emsg);

src/tree_schema_internal.h

@@ -181,14 +181,6 @@ struct lysp_yin_ctx {
     struct lyxml_ctx *xmlctx;        /**< context for xml parser */
 };
 
-/**
- * @brief Internal pattern hash table record.
- */
-struct ly_pattern_ht_rec {
-    const char *pattern;            /**< Pattern string, used both as key to hash and value to search for. */
-    pcre2_code *pcode;              /**< Compiled PCRE2 pattern code. */
-};
-
 /**
  * @brief Check that @p c is valid UTF8 code point for YANG string.
  *