Hi all, With db4f21e in place, there is no need to worry about explicitely freeing any regular expressions that may have been compiled when loading HBA or ident files because MemoryContextDelete() would be able to take care of that now that these are palloc'd (the definitions in regcustom.h superseed the ones of regguts.h).
The logic in hba.c that scans all the HBA and ident lines to any
regexps can be simplified a lot. Most of this code is new in 16~, so
I think that it is worth cleaning up this stuff now rather than wait
for 17 to open for business. Still, this is optional, and I don't
mind waiting for 17 if the regexp/palloc business proves to be an
issue during beta.
FWIW, one would see leaks in the postmaster process with files like
that on repeated SIGHUPs before db4f21e:
$ cat $PGDATA/pg_hba.conf
local "/^db\d{2,4}$" all trust
local "/^db\d{2,po}$" all trust
local "/^db\d{2,4}$" all trust
$ cat $PGDATA/pg_ident.conf
foo "/^user\d{2,po}$" bar
foo "/^uesr\d{2,4}$" bar
With this configuration, there are no leaks on SIGHUPs after db4f21e
as MemoryContextDelete() does all the job. Please see the attached.
Thoughts or opinions?
--
Michael
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index adedbd3128..d786a01835 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -95,11 +95,6 @@ static MemoryContext parsed_hba_context = NULL;
/*
* pre-parsed content of ident mapping file: list of IdentLine structs.
* parsed_ident_context is the memory context where it lives.
- *
- * NOTE: the IdentLine structs can contain AuthTokens with pre-compiled
- * regular expressions that live outside the memory context. Before
- * destroying or resetting the memory context, they need to be explicitly
- * free'd.
*/
static List *parsed_ident_lines = NIL;
static MemoryContext parsed_ident_context = NULL;
@@ -316,30 +311,6 @@ free_auth_token(AuthToken *token)
pg_regfree(token->regex);
}
-/*
- * Free a HbaLine. Its list of AuthTokens for databases and roles may include
- * regular expressions that need to be cleaned up explicitly.
- */
-static void
-free_hba_line(HbaLine *line)
-{
- ListCell *cell;
-
- foreach(cell, line->roles)
- {
- AuthToken *tok = lfirst(cell);
-
- free_auth_token(tok);
- }
-
- foreach(cell, line->databases)
- {
- AuthToken *tok = lfirst(cell);
-
- free_auth_token(tok);
- }
-}
-
/*
* Copy a AuthToken struct into freshly palloc'd memory.
*/
@@ -2722,30 +2693,14 @@ load_hba(void)
if (!ok)
{
/*
- * File contained one or more errors, so bail out, first being careful
- * to clean up whatever we allocated. Most stuff will go away via
- * MemoryContextDelete, but we have to clean up regexes explicitly.
+ * File contained one or more errors, so bail out. MemoryContextDelete
+ * is enough to clean up everything, including regexes.
*/
- foreach(line, new_parsed_lines)
- {
- HbaLine *newline = (HbaLine *) lfirst(line);
-
- free_hba_line(newline);
- }
MemoryContextDelete(hbacxt);
return false;
}
/* Loaded new file successfully, replace the one we use */
- if (parsed_hba_lines != NIL)
- {
- foreach(line, parsed_hba_lines)
- {
- HbaLine *newline = (HbaLine *) lfirst(line);
-
- free_hba_line(newline);
- }
- }
if (parsed_hba_context != NULL)
MemoryContextDelete(parsed_hba_context);
parsed_hba_context = hbacxt;
@@ -3044,8 +2999,7 @@ load_ident(void)
{
FILE *file;
List *ident_lines = NIL;
- ListCell *line_cell,
- *parsed_line_cell;
+ ListCell *line_cell;
List *new_parsed_lines = NIL;
bool ok = true;
MemoryContext oldcxt;
@@ -3102,30 +3056,14 @@ load_ident(void)
if (!ok)
{
/*
- * File contained one or more errors, so bail out, first being careful
- * to clean up whatever we allocated. Most stuff will go away via
- * MemoryContextDelete, but we have to clean up regexes explicitly.
+ * File contained one or more errors, so bail out. MemoryContextDelete
+ * is enough to clean up everything, including regexes.
*/
- foreach(parsed_line_cell, new_parsed_lines)
- {
- newline = (IdentLine *) lfirst(parsed_line_cell);
- free_auth_token(newline->system_user);
- free_auth_token(newline->pg_user);
- }
MemoryContextDelete(ident_context);
return false;
}
/* Loaded new file successfully, replace the one we use */
- if (parsed_ident_lines != NIL)
- {
- foreach(parsed_line_cell, parsed_ident_lines)
- {
- newline = (IdentLine *) lfirst(parsed_line_cell);
- free_auth_token(newline->system_user);
- free_auth_token(newline->pg_user);
- }
- }
if (parsed_ident_context != NULL)
MemoryContextDelete(parsed_ident_context);
signature.asc
Description: PGP signature
