Don't leak compiled regex(es) when an ispell cache entry is dropped.

The text search cache mechanisms assume that we can clean up
an invalidated dictionary cache entry simply by resetting the
associated long-lived memory context.  However, that does not work
for ispell affixes that make use of regular expressions, because
the regex library deals in plain old malloc.  Hence, we leaked
compiled regex(es) any time we dropped such a cache entry.  That
could quickly add up, since even a fairly trivial regex can use up
tens of kB, and a large one can eat megabytes.  Add a memory context
callback to ensure that a regex gets freed when its owning cache
entry is cleared.

Found via valgrind testing.
This problem is ancient, so back-patch to all supported branches.

Discussion: https://postgr.es/m/3816764.1616104288@sss.pgh.pa.us

diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index 32f6c5081e9515b3c5155448b5ad084fe6df8fee..34c91e049f3995cf55d8169fe2d8082e6c0b0be2 100644 (file)
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -654,6 +654,17 @@ FindWord(IspellDict *Conf, const char *word, char *affixflag, int flag)
        return 0;
 }
 
+/*
+ * Context reset/delete callback for a regular expression used in an affix
+ */
+static void
+regex_affix_deletion_callback(void *arg)
+{
+       aff_regex_struct *pregex = (aff_regex_struct *) arg;
+
+       pg_regfree(&(pregex->regex));
+}
+
 /*
  * Adds a new affix rule to the Affix field.
  *
@@ -716,6 +727,7 @@ NIAddAffix(IspellDict *Conf, const char *flag, char flagflags, const char *mask,
                int                     err;
                pg_wchar   *wmask;
                char       *tmask;
+               aff_regex_struct *pregex;
 
                Affix->issimple = 0;
                Affix->isregis = 0;
@@ -729,18 +741,32 @@ NIAddAffix(IspellDict *Conf, const char *flag, char flagflags, const char *mask,
                wmask = (pg_wchar *) tmpalloc((masklen + 1) * sizeof(pg_wchar));
                wmasklen = pg_mb2wchar_with_len(tmask, wmask, masklen);
 
-               err = pg_regcomp(&(Affix->reg.regex), wmask, wmasklen,
+               /*
+                * The regex engine stores its stuff using malloc not palloc, so we
+                * must arrange to explicitly clean up the regex when the dictionary's
+                * context is cleared.  That means the regex_t has to stay in a fixed
+                * location within the context; we can't keep it directly in the AFFIX
+                * struct, since we may sort and resize the array of AFFIXes.
+                */
+               Affix->reg.pregex = pregex = palloc(sizeof(aff_regex_struct));
+
+               err = pg_regcomp(&(pregex->regex), wmask, wmasklen,
                                                 REG_ADVANCED | REG_NOSUB,
                                                 DEFAULT_COLLATION_OID);
                if (err)
                {
                        char            errstr[100];
 
-                       pg_regerror(err, &(Affix->reg.regex), errstr, sizeof(errstr));
+                       pg_regerror(err, &(pregex->regex), errstr, sizeof(errstr));
                        ereport(ERROR,
                                        (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
                                         errmsg("invalid regular expression: %s", errstr)));
                }
+
+               pregex->mcallback.func = regex_affix_deletion_callback;
+               pregex->mcallback.arg = (void *) pregex;
+               MemoryContextRegisterResetCallback(CurrentMemoryContext,
+                                                                                  &pregex->mcallback);
        }
 
        Affix->flagflags = flagflags;
@@ -2119,7 +2145,6 @@ CheckAffix(const char *word, size_t len, AFFIX *Affix, int flagflags, char *neww
        }
        else
        {
-               int                     err;
                pg_wchar   *data;
                size_t          data_len;
                int                     newword_len;
@@ -2129,7 +2154,8 @@ CheckAffix(const char *word, size_t len, AFFIX *Affix, int flagflags, char *neww
                data = (pg_wchar *) palloc((newword_len + 1) * sizeof(pg_wchar));
                data_len = pg_mb2wchar_with_len(newword, data, newword_len);
 
-               if (!(err = pg_regexec(&(Affix->reg.regex), data, data_len, 0, NULL, 0, NULL, 0)))
+               if (pg_regexec(&(Affix->reg.pregex->regex), data, data_len,
+                                          0, NULL, 0, NULL, 0) == REG_OKAY)
                {
                        pfree(data);
                        return newword;

diff --git a/src/include/tsearch/dicts/spell.h b/src/include/tsearch/dicts/spell.h
index 3032d0b508747b2db91f6f5cf582273ac029b446..a3e206339b3432be75fc4d5f0fe3b1e46be70ead 100644 (file)
--- a/src/include/tsearch/dicts/spell.h
+++ b/src/include/tsearch/dicts/spell.h
@@ -81,6 +81,17 @@ typedef struct spell_struct
 
 #define SPELLHDRSZ     (offsetof(SPELL, word))
 
+/*
+ * If an affix uses a regex, we have to store that separately in a struct
+ * that won't move around when arrays of affixes are enlarged or sorted.
+ * This is so that it can be found to be cleaned up at context destruction.
+ */
+typedef struct aff_regex_struct
+{
+       regex_t         regex;
+       MemoryContextCallback mcallback;
+} aff_regex_struct;
+
 /*
  * Represents an entry in an affix list.
  */
@@ -97,7 +108,7 @@ typedef struct aff_struct
        char       *repl;
        union
        {
-               regex_t         regex;
+               aff_regex_struct *pregex;
                Regis           regis;
        }                       reg;
 } AFFIX;