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
+}