From ab02d702ef08343fba30d90fdf7df5950063e8c9 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 11 May 2022 15:27:33 -0400 Subject: [PATCH] Remove non-functional code for unloading loadable modules. The code for unloading a library has been commented-out for over 12 years, ever since commit 602a9ef5a7c60151e10293ae3c4bb3fbb0132d03, and we're no closer to supporting it now than we were back then. Nathan Bossart, reviewed by Michael Paquier and by me. Discussion: http://postgr.es/m/Ynsc9bRL1caUSBSE@paquier.xyz --- contrib/auto_explain/auto_explain.c | 14 ---- contrib/passwordcheck/passwordcheck.c | 11 --- .../pg_stat_statements/pg_stat_statements.c | 18 ----- 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(+), 200 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 df2ce63790..ceaad81a43 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -305,7 +305,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); @@ -481,23 +480,6 @@ _PG_init(void) ProcessUtility_hook = pgss_ProcessUtility; } -/* - * Module unload callback - */ -void -_PG_fini(void) -{ - /* Uninstall hooks. */ - 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_startup hook: allocate or attach to shared memory, * then load any pre-existing statistics from file. 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 */