From a28269708844246fb1ec00a536b391cac0a64972 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 14 Apr 2023 07:27:44 +0900 Subject: [PATCH] Remove code in charge of freeing regexps generation by Lab.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bea3d7e has redesigned the regexp engine so as all the allocations go through palloc() with a dedicated memory context. hba.c had to cope with the past memory management logic by going through all the HBA and ident lines generated, then directly free all the regexps found in AuthTokens to ensure that no leaks would happen. Such leaks could happen for example in the postmaster after a SIGHUP, in the event of an HBA and/or ident reload failure where all the new content parsed must be discarded, including all the regexps that may have been compiled. Now that regexps are palloc()'d in their own memory context, MemoryContextDelete() is enough to ensure that all the compiled regexps are properly gone. Simplifying this logic in hba.c has the effect to only remove code. Most of it is new in v16, except the part for regexps compiled in ident entries for the system username, so doing this cleanup now rather than when v17 opens for business will reduce future diffs with the upcoming REL_16_STABLE. Some comments were incorrect since bea3d7e, now fixed to reflect the reality. Reviewed-by: Bertrand Drouvot, Álvaro Herrera Discussion: https://postgr.es/m/ZDdJ289Ky2qEj4h+@paquier.xyz --- src/backend/libpq/hba.c | 72 +++-------------------------------------- 1 file changed, 5 insertions(+), 67 deletions(-) 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);