Subject: [PATCH v8 1/2] Remove logic for unloading dynamically loaded files.

From: Nathan Bossart <[email-masked]>
Date: Fri, 6 May 2022 13:53:54 -0700

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   | 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;
    <indexterm zone="xfunc-c-dynload">
     <primary>_PG_init</primary>
    </indexterm>
-   <indexterm zone="xfunc-c-dynload">
-    <primary>_PG_fini</primary>
-   </indexterm>
    <indexterm zone="xfunc-c-dynload">
     <primary>library initialization function</primary>
    </indexterm>
-   <indexterm zone="xfunc-c-dynload">
-    <primary>library finalization function</primary>
-   </indexterm>
 
    <para>
-    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
     <function>_PG_init</function>, 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
-    <function>_PG_fini</function>, that function will be called immediately before
-    unloading the file.  Likewise, the function receives no parameters and
-    should return void.  Note that <function>_PG_fini</function> 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.
    </para>
 
   </sect2>
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

--wac7ysb48OaltWcw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v8-0002-Add-a-new-shmem_request_hook-hook.patch"