From 7949bda7adcb378f8355f06325d4fade56077337 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 23 Nov 2022 14:39:07 +0900 Subject: [PATCH v21 1/2] Rework memory contexts in charge of HBA/ident tokenization The list of TokenizedAuthLines generated for the tokens are now stored in a static context called tokenize_context, where only all the parsed elements are stored. This context is stored when opening the top-root of an authentication file, and is cleaned up once we are done with it with a new routine called free_auth_file(). One call of open_auth_file() should have one matching call of free_auth_file(). Rather than having tokenize_auth_file() return a memory context that includes all the records, this now creates and drops one memory context each time the function is called. This will simplify recursive calls to this routine for the upcoming inclusion record logic. While on it, rename tokenize_inc_file() to tokenize_expand_file() as this would conflict with the upcoming patch that will add inclusion records for HBA/ident files. Reloading HBA/indent configuration in a tight loop shows no leaks, as of one type of test done (with and without -DEXEC_BACKEND). --- src/include/libpq/hba.h | 5 +- src/backend/libpq/hba.c | 152 +++++++++++++++++++++++-------- src/backend/utils/adt/hbafuncs.c | 12 +-- 3 files changed, 119 insertions(+), 50 deletions(-) diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h index a84a5f0961..90c51ad6fa 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -179,7 +179,8 @@ extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel); extern bool pg_isblank(const char c); extern FILE *open_auth_file(const char *filename, int elevel, int depth, char **err_msg); -extern MemoryContext tokenize_auth_file(const char *filename, FILE *file, - List **tok_lines, int elevel, int depth); +extern void free_auth_file(FILE *file, int depth); +extern void tokenize_auth_file(const char *filename, FILE *file, + List **tok_lines, int elevel, int depth); #endif /* HBA_H */ diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index abdebeb3f8..9064ad6714 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -76,6 +76,13 @@ typedef struct #define token_is_keyword(t, k) (!t->quoted && strcmp(t->string, k) == 0) #define token_matches(t, k) (strcmp(t->string, k) == 0) +/* + * Memory context holding the list of TokenizedAuthLines when parsing + * HBA or ident config files. This is created when loading the top HBA + + or ident file (depth of 0). + */ +static MemoryContext tokenize_context = NULL; + /* * pre-parsed content of HBA config file: list of HbaLine structs. * parsed_hba_context is the memory context where it lives. @@ -121,9 +128,9 @@ static const char *const UserAuthName[] = }; -static List *tokenize_inc_file(List *tokens, const char *outer_filename, - const char *inc_filename, int elevel, - int depth, char **err_msg); +static List *tokenize_expand_file(List *tokens, const char *outer_filename, + const char *inc_filename, int elevel, + int depth, char **err_msg); static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int elevel, char **err_msg); static int regcomp_auth_token(AuthToken *token, char *filename, int line_num, @@ -437,17 +444,27 @@ next_field_expand(const char *filename, char **lineptr, /* Is this referencing a file? */ if (!initial_quote && buf[0] == '@' && buf[1] != '\0') - tokens = tokenize_inc_file(tokens, filename, buf + 1, - elevel, depth + 1, err_msg); + tokens = tokenize_expand_file(tokens, filename, buf + 1, + elevel, depth + 1, err_msg); else + { + MemoryContext oldcxt; + + /* + * lappend() may do its own allocations, so move to the context + * for the list of tokens. + */ + oldcxt = MemoryContextSwitchTo(tokenize_context); tokens = lappend(tokens, make_auth_token(buf, initial_quote)); + MemoryContextSwitchTo(oldcxt); + } } while (trailing_comma && (*err_msg == NULL)); return tokens; } /* - * tokenize_inc_file + * tokenize_expand_file * Expand a file included from another file into an hba "field" * * Opens and tokenises a file included from another HBA config file with @, @@ -462,18 +479,17 @@ next_field_expand(const char *filename, char **lineptr, * there was an error. */ static List * -tokenize_inc_file(List *tokens, - const char *outer_filename, - const char *inc_filename, - int elevel, - int depth, - char **err_msg) +tokenize_expand_file(List *tokens, + const char *outer_filename, + const char *inc_filename, + int elevel, + int depth, + char **err_msg) { char *inc_fullname; FILE *inc_file; List *inc_lines; ListCell *inc_line; - MemoryContext linecxt; inc_fullname = AbsoluteConfigLocation(inc_filename, outer_filename); inc_file = open_auth_file(inc_fullname, elevel, depth, err_msg); @@ -486,13 +502,15 @@ tokenize_inc_file(List *tokens, } /* There is possible recursion here if the file contains @ */ - linecxt = tokenize_auth_file(inc_fullname, inc_file, &inc_lines, elevel, - depth); + tokenize_auth_file(inc_fullname, inc_file, &inc_lines, elevel, + depth); - FreeFile(inc_file); pfree(inc_fullname); - /* Copy all tokens found in the file and append to the tokens list */ + /* + * Move all the tokens found in the file to the tokens list. These are + * already saved in tokenize_context. + */ foreach(inc_line, inc_lines) { TokenizedAuthLine *tok_line = (TokenizedAuthLine *) lfirst(inc_line); @@ -513,16 +531,40 @@ tokenize_inc_file(List *tokens, foreach(inc_token, inc_tokens) { AuthToken *token = lfirst(inc_token); + MemoryContext oldcxt; - tokens = lappend(tokens, copy_auth_token(token)); + /* + * lappend() may do its own allocations, so move to the + * context for the list of tokens. + */ + oldcxt = MemoryContextSwitchTo(tokenize_context); + tokens = lappend(tokens, token); + MemoryContextSwitchTo(oldcxt); } } } - MemoryContextDelete(linecxt); + free_auth_file(inc_file, depth); return tokens; } +/* + * free_auth_file + * Free a file opened by open_auth_file(). + */ +void +free_auth_file(FILE *file, int depth) +{ + FreeFile(file); + + /* If this is the last cleanup, remove the tokenization context */ + if (depth == 0) + { + MemoryContextDelete(tokenize_context); + tokenize_context = NULL; + } +} + /* * open_auth_file * Open the given file. @@ -558,6 +600,22 @@ open_auth_file(const char *filename, int elevel, int depth, return NULL; } + /* + * When opening the top-level file, create the memory context used for the + * tokenization. This will be closed with this file when coming back to + * this level of cleanup. + */ + if (depth == 0) + { + /* + * A context may be present, but assume that it has been eliminated + * already. + */ + tokenize_context = AllocSetContextCreate(CurrentMemoryContext, + "tokenize_context", + ALLOCSET_START_SMALL_SIZES); + } + file = AllocateFile(filename, "r"); if (file == NULL) { @@ -570,6 +628,8 @@ open_auth_file(const char *filename, int elevel, int depth, if (err_msg) *err_msg = psprintf("could not open file \"%s\": %s", filename, strerror(save_errno)); + /* the caller may care about some specific errno */ + errno = save_errno; return NULL; } @@ -593,32 +653,34 @@ tokenize_error_callback(void *arg) * Tokenize the given file. * * The output is a list of TokenizedAuthLine structs; see the struct definition - * in libpq/hba.h. + * in libpq/hba.h. This is the central pieces in charge of parsing the + * authentication files. All the operations of this function happen in its own + * local memory context, easing the cleanup of anything allocated locally. * * filename: the absolute path to the target file * file: the already-opened target file - * tok_lines: receives output list + * tok_lines: receives output list, saved into tokenize_context * elevel: message logging level * depth: level of recursion when tokenizing the target file * * Errors are reported by logging messages at ereport level elevel and by * adding TokenizedAuthLine structs containing non-null err_msg fields to the * output list. - * - * Return value is a memory context which contains all memory allocated by - * this function (it's a child of caller's context). */ -MemoryContext +void tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, int elevel, int depth) { - int line_number = 1; StringInfoData buf; + int line_number = 1; MemoryContext linecxt; + MemoryContext funccxt; MemoryContext oldcxt; ErrorContextCallback tokenerrcontext; tokenize_error_callback_arg callback_arg; + Assert(tokenize_context); + callback_arg.filename = filename; callback_arg.linenum = line_number; @@ -627,14 +689,19 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, tokenerrcontext.previous = error_context_stack; error_context_stack = &tokenerrcontext; + /* + * Do all the local tokenization in its own context, to ease the cleanup + * of any memory allocated while tokenizing. + */ linecxt = AllocSetContextCreate(CurrentMemoryContext, "tokenize_auth_file", ALLOCSET_SMALL_SIZES); - oldcxt = MemoryContextSwitchTo(linecxt); + funccxt = MemoryContextSwitchTo(linecxt); initStringInfo(&buf); - *tok_lines = NIL; + if (depth == 0) + *tok_lines = NIL; while (!feof(file) && !ferror(file)) { @@ -694,7 +761,15 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, elevel, depth, &err_msg); /* add field to line, unless we are at EOL or comment start */ if (current_field != NIL) + { + /* + * lappend() may do its own allocations, so move to the + * context for the list of tokens. + */ + oldcxt = MemoryContextSwitchTo(tokenize_context); current_line = lappend(current_line, current_field); + MemoryContextSwitchTo(oldcxt); + } } /* @@ -704,24 +779,25 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, { TokenizedAuthLine *tok_line; + oldcxt = MemoryContextSwitchTo(tokenize_context); tok_line = (TokenizedAuthLine *) palloc(sizeof(TokenizedAuthLine)); tok_line->fields = current_line; tok_line->file_name = pstrdup(filename); tok_line->line_num = line_number; tok_line->raw_line = pstrdup(buf.data); - tok_line->err_msg = err_msg; + tok_line->err_msg = err_msg ? pstrdup(err_msg) : NULL; *tok_lines = lappend(*tok_lines, tok_line); + MemoryContextSwitchTo(oldcxt); } line_number += continuations + 1; callback_arg.linenum = line_number; } - MemoryContextSwitchTo(oldcxt); + MemoryContextSwitchTo(funccxt); + MemoryContextDelete(linecxt); error_context_stack = tokenerrcontext.previous; - - return linecxt; } @@ -2409,7 +2485,6 @@ load_hba(void) ListCell *line; List *new_parsed_lines = NIL; bool ok = true; - MemoryContext linecxt; MemoryContext oldcxt; MemoryContext hbacxt; @@ -2420,8 +2495,7 @@ load_hba(void) return false; } - linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, LOG, 0); - FreeFile(file); + tokenize_auth_file(HbaFileName, file, &hba_lines, LOG, 0); /* Now parse all the lines */ Assert(PostmasterContext); @@ -2472,7 +2546,7 @@ load_hba(void) } /* Free tokenizer memory */ - MemoryContextDelete(linecxt); + free_auth_file(file, 0); MemoryContextSwitchTo(oldcxt); if (!ok) @@ -2776,7 +2850,6 @@ load_ident(void) *parsed_line_cell; List *new_parsed_lines = NIL; bool ok = true; - MemoryContext linecxt; MemoryContext oldcxt; MemoryContext ident_context; IdentLine *newline; @@ -2789,8 +2862,7 @@ load_ident(void) return false; } - linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, LOG, 0); - FreeFile(file); + tokenize_auth_file(IdentFileName, file, &ident_lines, LOG, 0); /* Now parse all the lines */ Assert(PostmasterContext); @@ -2826,7 +2898,7 @@ load_ident(void) } /* Free tokenizer memory */ - MemoryContextDelete(linecxt); + free_auth_file(file, 0); MemoryContextSwitchTo(oldcxt); if (!ok) diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c index b662e7b55f..87996da487 100644 --- a/src/backend/utils/adt/hbafuncs.c +++ b/src/backend/utils/adt/hbafuncs.c @@ -370,7 +370,6 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) List *hba_lines = NIL; ListCell *line; int rule_number = 0; - MemoryContext linecxt; MemoryContext hbacxt; MemoryContext oldcxt; @@ -382,8 +381,7 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) */ file = open_auth_file(HbaFileName, ERROR, 0, NULL); - linecxt = tokenize_auth_file(HbaFileName, file, &hba_lines, DEBUG3, 0); - FreeFile(file); + tokenize_auth_file(HbaFileName, file, &hba_lines, DEBUG3, 0); /* Now parse all the lines */ hbacxt = AllocSetContextCreate(CurrentMemoryContext, @@ -408,7 +406,7 @@ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) } /* Free tokenizer memory */ - MemoryContextDelete(linecxt); + free_auth_file(file, 0); /* Free parse_hba_line memory */ MemoryContextSwitchTo(oldcxt); MemoryContextDelete(hbacxt); @@ -514,7 +512,6 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) List *ident_lines = NIL; ListCell *line; int map_number = 0; - MemoryContext linecxt; MemoryContext identcxt; MemoryContext oldcxt; @@ -526,8 +523,7 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) */ file = open_auth_file(IdentFileName, ERROR, 0, NULL); - linecxt = tokenize_auth_file(IdentFileName, file, &ident_lines, DEBUG3, 0); - FreeFile(file); + tokenize_auth_file(IdentFileName, file, &ident_lines, DEBUG3, 0); /* Now parse all the lines */ identcxt = AllocSetContextCreate(CurrentMemoryContext, @@ -553,7 +549,7 @@ fill_ident_view(Tuplestorestate *tuple_store, TupleDesc tupdesc) } /* Free tokenizer memory */ - MemoryContextDelete(linecxt); + free_auth_file(file, 0); /* Free parse_ident_line memory */ MemoryContextSwitchTo(oldcxt); MemoryContextDelete(identcxt); -- 2.38.1