Suppress complaints about leaks in TS dictionary loading.

Like the situation with function cache loading, text search
dictionary loading functions tend to leak some cruft into the
dictionary's long-lived cache context.  To judge by the examples in
the core regression tests, not very many bytes are at stake.
Moreover, I don't see a way to prevent such leaks without changing the
API for TS template initialization functions: right now they do not
have to worry about making sure that their results are long-lived.

Hence, I think we should install a suppression rule rather than trying
to fix this completely.  However, I did grab some low-hanging fruit:
several places were leaking the result of get_tsearch_config_filename.
This seems worth doing mostly because they are inconsistent with other
dictionaries that were freeing it already.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us

diff --git a/src/backend/tsearch/dict_ispell.c b/src/backend/tsearch/dict_ispell.c
index 63bd193a78a894b3bff1080e9147882236a31066..debfbf956cc1f86990981646b5770f017daba92e 100644 (file)
--- a/src/backend/tsearch/dict_ispell.c
+++ b/src/backend/tsearch/dict_ispell.c
@@ -47,24 +47,30 @@ dispell_init(PG_FUNCTION_ARGS)
 
        if (strcmp(defel->defname, "dictfile") == 0)
        {
+           char       *filename;
+
            if (dictloaded)
                ereport(ERROR,
                        (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                         errmsg("multiple DictFile parameters")));
-           NIImportDictionary(&(d->obj),
-                              get_tsearch_config_filename(defGetString(defel),
-                                                          "dict"));
+           filename = get_tsearch_config_filename(defGetString(defel),
+                                                  "dict");
+           NIImportDictionary(&(d->obj), filename);
+           pfree(filename);
            dictloaded = true;
        }
        else if (strcmp(defel->defname, "afffile") == 0)
        {
+           char       *filename;
+
            if (affloaded)
                ereport(ERROR,
                        (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                         errmsg("multiple AffFile parameters")));
-           NIImportAffixes(&(d->obj),
-                           get_tsearch_config_filename(defGetString(defel),
-                                                       "affix"));
+           filename = get_tsearch_config_filename(defGetString(defel),
+                                                  "affix");
+           NIImportAffixes(&(d->obj), filename);
+           pfree(filename);
            affloaded = true;
        }
        else if (strcmp(defel->defname, "stopwords") == 0)

diff --git a/src/backend/tsearch/dict_synonym.c b/src/backend/tsearch/dict_synonym.c
index 0da5a9d686802bf8b666de9ad82b632e23141dc5..c2773eb01adee8e5053b0a551f681de6e622bcbf 100644 (file)
--- a/src/backend/tsearch/dict_synonym.c
+++ b/src/backend/tsearch/dict_synonym.c
@@ -199,6 +199,7 @@ skipline:
    }
 
    tsearch_readline_end(&trst);
+   pfree(filename);
 
    d->len = cur;
    qsort(d->syn, d->len, sizeof(Syn), compareSyn);

diff --git a/src/backend/tsearch/dict_thesaurus.c b/src/backend/tsearch/dict_thesaurus.c
index 1bebe36a6910e9f84deb836f7a19bd49b948ae04..1e6bbde1ca7d8bbd34da635edf3187eb19e8388a 100644 (file)
--- a/src/backend/tsearch/dict_thesaurus.c
+++ b/src/backend/tsearch/dict_thesaurus.c
@@ -167,17 +167,17 @@ addWrd(DictThesaurus *d, char *b, char *e, uint32 idsubst, uint16 nwrd, uint16 p
 static void
 thesaurusRead(const char *filename, DictThesaurus *d)
 {
+   char       *real_filename = get_tsearch_config_filename(filename, "ths");
    tsearch_readline_state trst;
    uint32      idsubst = 0;
    bool        useasis = false;
    char       *line;
 
-   filename = get_tsearch_config_filename(filename, "ths");
-   if (!tsearch_readline_begin(&trst, filename))
+   if (!tsearch_readline_begin(&trst, real_filename))
        ereport(ERROR,
                (errcode(ERRCODE_CONFIG_FILE_ERROR),
                 errmsg("could not open thesaurus file \"%s\": %m",
-                       filename)));
+                       real_filename)));
 
    while ((line = tsearch_readline(&trst)) != NULL)
    {
@@ -297,6 +297,7 @@ thesaurusRead(const char *filename, DictThesaurus *d)
    d->nsubst = idsubst;
 
    tsearch_readline_end(&trst);
+   pfree(real_filename);
 }
 
 static TheLexeme *

diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 18cccd778fd8c93652d3ee7f715a28027c7da687..e8ae53238d07a3b7ece3625106ce27cf1a27857b 100644 (file)
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -321,7 +321,9 @@ lookup_ts_dictionary_cache(Oid dictId)
 
            /*
             * Init method runs in dictionary's private memory context, and we
-            * make sure the options are stored there too
+            * make sure the options are stored there too.  This typically
+            * results in a small amount of memory leakage, but it's not worth
+            * complicating the API for tmplinit functions to avoid it.
             */
            oldcontext = MemoryContextSwitchTo(entry->dictCtx);
 

diff --git a/src/tools/valgrind.supp b/src/tools/valgrind.supp
index fad20c8f708049b5c302ca0bc806a3653deff810..3880007dfb3bbaf3d2c52e85370a586437091a01 100644 (file)
--- a/src/tools/valgrind.supp
+++ b/src/tools/valgrind.supp
@@ -215,3 +215,15 @@
    ...
    fun:cached_function_compile
 }
+
+# Suppress complaints about stuff leaked during TS dictionary loading.
+# Not very much is typically lost there, and preventing it would
+# require a risky API change for TS tmplinit functions.
+{
+   hide_ts_dictionary_leaks
+   Memcheck:Leak
+   match-leak-kinds: definite,possible,indirect
+
+   ...
+   fun:lookup_ts_dictionary_cache
+}