From a2471fd16262ffa30f3278360d7d9a8c41b67d3d Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Fri, 6 May 2022 13:53:54 -0700 Subject: [PATCH v7 2/2] Remove logic for unloading dynamically loaded files. The code for unloading a library has been commented-out for over 12 years (602a9ef), and we're no closer to supporting it now than we were back then. --- contrib/auto_explain/auto_explain.c | 14 ---- contrib/passwordcheck/passwordcheck.c | 11 --- .../pg_stat_statements/pg_stat_statements.c | 19 ----- doc/src/sgml/xfunc.sgml | 18 +---- src/backend/postmaster/pgarch.c | 2 +- src/backend/utils/fmgr/dfmgr.c | 77 ++----------------- src/backend/utils/fmgr/fmgr.c | 14 ---- src/include/fmgr.h | 1 - src/pl/plpgsql/src/plpgsql.h | 2 - .../modules/delay_execution/delay_execution.c | 10 +-- .../ssl_passphrase_func.c | 7 -- .../modules/test_oat_hooks/test_oat_hooks.c | 22 +----- .../modules/test_rls_hooks/test_rls_hooks.c | 17 ---- 13 files changed, 13 insertions(+), 201 deletions(-) diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c index d3029f85ef..c9a0d947c8 100644 --- a/contrib/auto_explain/auto_explain.c +++ b/contrib/auto_explain/auto_explain.c @@ -77,7 +77,6 @@ static ExecutorFinish_hook_type prev_ExecutorFinish = NULL; static ExecutorEnd_hook_type prev_ExecutorEnd = NULL; void _PG_init(void); -void _PG_fini(void); static void explain_ExecutorStart(QueryDesc *queryDesc, int eflags); static void explain_ExecutorRun(QueryDesc *queryDesc, @@ -244,19 +243,6 @@ _PG_init(void) ExecutorEnd_hook = explain_ExecutorEnd; } -/* - * Module unload callback - */ -void -_PG_fini(void) -{ - /* Uninstall hooks. */ - ExecutorStart_hook = prev_ExecutorStart; - ExecutorRun_hook = prev_ExecutorRun; - ExecutorFinish_hook = prev_ExecutorFinish; - ExecutorEnd_hook = prev_ExecutorEnd; -} - /* * ExecutorStart hook: start up logging if needed */ diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c index 074836336d..9d8c58ded0 100644 --- a/contrib/passwordcheck/passwordcheck.c +++ b/contrib/passwordcheck/passwordcheck.c @@ -33,7 +33,6 @@ static check_password_hook_type prev_check_password_hook = NULL; #define MIN_PWD_LENGTH 8 extern void _PG_init(void); -extern void _PG_fini(void); /* * check_password @@ -149,13 +148,3 @@ _PG_init(void) prev_check_password_hook = check_password_hook; check_password_hook = check_password; } - -/* - * Module unload function - */ -void -_PG_fini(void) -{ - /* uninstall hook */ - check_password_hook = prev_check_password_hook; -} diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 87b75d779e..9ea887d887 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -306,7 +306,6 @@ static bool pgss_save; /* whether to save stats across shutdown */ /*---- Function declarations ----*/ void _PG_init(void); -void _PG_fini(void); PG_FUNCTION_INFO_V1(pg_stat_statements_reset); PG_FUNCTION_INFO_V1(pg_stat_statements_reset_1_7); @@ -477,24 +476,6 @@ _PG_init(void) ProcessUtility_hook = pgss_ProcessUtility; } -/* - * Module unload callback - */ -void -_PG_fini(void) -{ - /* Uninstall hooks. */ - shmem_request_hook = prev_shmem_request_hook; - shmem_startup_hook = prev_shmem_startup_hook; - post_parse_analyze_hook = prev_post_parse_analyze_hook; - planner_hook = prev_planner_hook; - ExecutorStart_hook = prev_ExecutorStart; - ExecutorRun_hook = prev_ExecutorRun; - ExecutorFinish_hook = prev_ExecutorFinish; - ExecutorEnd_hook = prev_ExecutorEnd; - ProcessUtility_hook = prev_ProcessUtility; -} - /* * shmem_request hook: request additional shared resources. We'll allocate or * attach to the shared resources in pgss_shmem_startup(). diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 07c5fd198b..fbe718e3c2 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -1978,28 +1978,16 @@ PG_MODULE_MAGIC; _PG_init - - _PG_fini - library initialization function - - library finalization function - - Optionally, a dynamically loaded file can contain initialization and - finalization functions. If the file includes a function named + Optionally, a dynamically loaded file can contain an initialization + function. If the file includes a function named _PG_init, that function will be called immediately after loading the file. The function receives no parameters and should - return void. If the file includes a function named - _PG_fini, that function will be called immediately before - unloading the file. Likewise, the function receives no parameters and - should return void. Note that _PG_fini will only be called - during an unload of the file, not during process termination. - (Presently, unloads are disabled and will never occur, but this may - change in the future.) + return void. There is presently no way to unload a dynamically loaded file. diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 0c8ca29f73..8beff4a53c 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -802,7 +802,7 @@ HandlePgArchInterrupts(void) * Ideally, we would simply unload the previous archive module and * load the new one, but there is presently no mechanism for * unloading a library (see the comment above - * internal_unload_library()). To deal with this, we simply restart + * internal_load_library()). To deal with this, we simply restart * the archiver. The new archive module will be loaded when the new * archiver process starts up. */ diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c index 3774f33e0e..7f9ea97280 100644 --- a/src/backend/utils/fmgr/dfmgr.c +++ b/src/backend/utils/fmgr/dfmgr.c @@ -37,9 +37,8 @@ #include "utils/hsearch.h" -/* signatures for PostgreSQL-specific library init/fini functions */ +/* signature for PostgreSQL-specific library init function */ typedef void (*PG_init_t) (void); -typedef void (*PG_fini_t) (void); /* hashtable entry for rendezvous variables */ typedef struct @@ -79,7 +78,6 @@ char *Dynamic_library_path; static void *internal_load_library(const char *libname); static void incompatible_module_error(const char *libname, const Pg_magic_struct *module_magic_data) pg_attribute_noreturn(); -static void internal_unload_library(const char *libname); static bool file_exists(const char *name); static char *expand_dynamic_library_name(const char *name); static void check_restricted_library_name(const char *name); @@ -154,9 +152,6 @@ load_file(const char *filename, bool restricted) /* Expand the possibly-abbreviated filename to an exact path name */ fullname = expand_dynamic_library_name(filename); - /* Unload the library if currently loaded */ - internal_unload_library(fullname); - /* Load the shared library */ (void) internal_load_library(fullname); @@ -179,6 +174,11 @@ lookup_external_function(void *filehandle, const char *funcname) * loaded. Return the pg_dl* handle for the file. * * Note: libname is expected to be an exact name for the library file. + * + * NB: There is presently no way to unload a dynamically loaded file. We might + * add one someday if we can convince ourselves we have safe protocols for un- + * hooking from hook function pointers, releasing custom GUC variables, and + * perhaps other things that are definitely unsafe currently. */ static void * internal_load_library(const char *libname) @@ -400,71 +400,6 @@ incompatible_module_error(const char *libname, errdetail_internal("%s", details.data))); } -/* - * Unload the specified dynamic-link library file, if it is loaded. - * - * Note: libname is expected to be an exact name for the library file. - * - * XXX for the moment, this is disabled, resulting in LOAD of an already-loaded - * library always being a no-op. We might re-enable it someday if we can - * convince ourselves we have safe protocols for un-hooking from hook function - * pointers, releasing custom GUC variables, and perhaps other things that - * are definitely unsafe currently. - */ -static void -internal_unload_library(const char *libname) -{ -#ifdef NOT_USED - DynamicFileList *file_scanner, - *prv, - *nxt; - struct stat stat_buf; - PG_fini_t PG_fini; - - /* - * We need to do stat() in order to determine whether this is the same - * file as a previously loaded file; it's also handy so as to give a good - * error message if bogus file name given. - */ - if (stat(libname, &stat_buf) == -1) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not access file \"%s\": %m", libname))); - - /* - * We have to zap all entries in the list that match on either filename or - * inode, else internal_load_library() will still think it's present. - */ - prv = NULL; - for (file_scanner = file_list; file_scanner != NULL; file_scanner = nxt) - { - nxt = file_scanner->next; - if (strcmp(libname, file_scanner->filename) == 0 || - SAME_INODE(stat_buf, *file_scanner)) - { - if (prv) - prv->next = nxt; - else - file_list = nxt; - - /* - * If the library has a _PG_fini() function, call it. - */ - PG_fini = (PG_fini_t) dlsym(file_scanner->handle, "_PG_fini"); - if (PG_fini) - (*PG_fini) (); - - clear_external_function_hash(file_scanner->handle); - dlclose(file_scanner->handle); - free((char *) file_scanner); - /* prv does not change */ - } - else - prv = file_scanner; - } -#endif /* NOT_USED */ -} - static bool file_exists(const char *name) { diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index b2e72b3243..a9dd068095 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -582,20 +582,6 @@ record_C_func(HeapTuple procedureTuple, entry->inforec = inforec; } -/* - * clear_external_function_hash: remove entries for a library being closed - * - * Presently we just zap the entire hash table, but later it might be worth - * the effort to remove only the entries associated with the given handle. - */ -void -clear_external_function_hash(void *filehandle) -{ - if (CFuncHash) - hash_destroy(CFuncHash); - CFuncHash = NULL; -} - /* * Copy an FmgrInfo struct diff --git a/src/include/fmgr.h b/src/include/fmgr.h index a1cf4bd646..d55abc5414 100644 --- a/src/include/fmgr.h +++ b/src/include/fmgr.h @@ -705,7 +705,6 @@ extern bytea *OidSendFunctionCall(Oid functionId, Datum val); * Routines in fmgr.c */ extern const Pg_finfo_record *fetch_finfo_record(void *filehandle, const char *funcname); -extern void clear_external_function_hash(void *filehandle); extern Oid fmgr_internal_function(const char *proname); extern Oid get_fn_expr_rettype(FmgrInfo *flinfo); extern Oid get_fn_expr_argtype(FmgrInfo *flinfo, int argnum); diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 6444347ce9..4e6ee1c619 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -1100,8 +1100,6 @@ typedef struct PLpgSQL_execstate * variable "PLpgSQL_plugin" and set it to point to a PLpgSQL_plugin struct. * Typically the struct could just be static data in the plugin library. * We expect that a plugin would do this at library load time (_PG_init()). - * It must also be careful to set the rendezvous variable back to NULL - * if it is unloaded (_PG_fini()). * * This structure is basically a collection of function pointers --- at * various interesting points in pl_exec.c, we call these functions diff --git a/src/test/modules/delay_execution/delay_execution.c b/src/test/modules/delay_execution/delay_execution.c index cf34e8c2d7..407ebc0eda 100644 --- a/src/test/modules/delay_execution/delay_execution.c +++ b/src/test/modules/delay_execution/delay_execution.c @@ -36,9 +36,8 @@ static int post_planning_lock_id = 0; /* Save previous planner hook user to be a good citizen */ static planner_hook_type prev_planner_hook = NULL; -/* Module load/unload functions */ +/* Module load function */ void _PG_init(void); -void _PG_fini(void); /* planner_hook function to provide the desired delay */ @@ -97,10 +96,3 @@ _PG_init(void) prev_planner_hook = planner_hook; planner_hook = delay_execution_planner; } - -/* Module unload function (pro forma, not used currently) */ -void -_PG_fini(void) -{ - planner_hook = prev_planner_hook; -} diff --git a/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c b/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c index b5bb5580a0..e9f2329a5a 100644 --- a/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c +++ b/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c @@ -21,7 +21,6 @@ PG_MODULE_MAGIC; void _PG_init(void); -void _PG_fini(void); static char *ssl_passphrase = NULL; @@ -55,12 +54,6 @@ _PG_init(void) openssl_tls_init_hook = set_rot13; } -void -_PG_fini(void) -{ - /* do nothing yet */ -} - static void set_rot13(SSL_CTX *context, bool isServerStart) { diff --git a/src/test/modules/test_oat_hooks/test_oat_hooks.c b/src/test/modules/test_oat_hooks/test_oat_hooks.c index 551da5d498..6f9838f93b 100644 --- a/src/test/modules/test_oat_hooks/test_oat_hooks.c +++ b/src/test/modules/test_oat_hooks/test_oat_hooks.c @@ -43,7 +43,7 @@ static bool REGRESS_userset_variable2 = false; static bool REGRESS_suset_variable1 = false; static bool REGRESS_suset_variable2 = false; -/* Saved hook values in case of unload */ +/* Saved hook values */ static object_access_hook_type next_object_access_hook = NULL; static object_access_hook_type_str next_object_access_hook_str = NULL; static ExecutorCheckPerms_hook_type next_exec_check_perms_hook = NULL; @@ -70,10 +70,9 @@ static char *accesstype_arg_to_string(ObjectAccessType access, void *arg); void _PG_init(void); -void _PG_fini(void); /* - * Module load/unload callback + * Module load callback */ void _PG_init(void) @@ -231,23 +230,6 @@ _PG_init(void) ProcessUtility_hook = REGRESS_utility_command; } -void -_PG_fini(void) -{ - /* Unload hooks */ - if (object_access_hook == REGRESS_object_access_hook) - object_access_hook = next_object_access_hook; - - if (object_access_hook_str == REGRESS_object_access_hook_str) - object_access_hook_str = next_object_access_hook_str; - - if (ExecutorCheckPerms_hook == REGRESS_exec_check_perms) - ExecutorCheckPerms_hook = next_exec_check_perms_hook; - - if (ProcessUtility_hook == REGRESS_utility_command) - ProcessUtility_hook = next_ProcessUtility_hook; -} - static void emit_audit_message(const char *type, const char *hook, char *action, char *objName) { diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c index 4f862d081b..b8e0aa2d0b 100644 --- a/src/test/modules/test_rls_hooks/test_rls_hooks.c +++ b/src/test/modules/test_rls_hooks/test_rls_hooks.c @@ -29,34 +29,17 @@ PG_MODULE_MAGIC; -/* Saved hook values in case of unload */ -static row_security_policy_hook_type prev_row_security_policy_hook_permissive = NULL; -static row_security_policy_hook_type prev_row_security_policy_hook_restrictive = NULL; - void _PG_init(void); -void _PG_fini(void); /* Install hooks */ void _PG_init(void) { - /* Save values for unload */ - prev_row_security_policy_hook_permissive = row_security_policy_hook_permissive; - prev_row_security_policy_hook_restrictive = row_security_policy_hook_restrictive; - /* Set our hooks */ row_security_policy_hook_permissive = test_rls_hooks_permissive; row_security_policy_hook_restrictive = test_rls_hooks_restrictive; } -/* Uninstall hooks */ -void -_PG_fini(void) -{ - row_security_policy_hook_permissive = prev_row_security_policy_hook_permissive; - row_security_policy_hook_restrictive = prev_row_security_policy_hook_restrictive; -} - /* * Return permissive policies to be added */ -- 2.25.1