[SV 63219] Support an "unload" function for loaded objects

If a loaded object defines a symbol <object>_gmk_unload, assume it's
a function and invoke it whenever the loaded object is unloaded.
Original implementation by Dmitry Goncharov <dgoncharov@users.sf.net>

* NEWS: Announce the change.
* doc/make.texi: Describe the behavior.
* src/gnumake.h: Add information to the comments.
* src/makeint.h (unload_all): Declare a new function.
* src/main.c (die): Invoke unload_all().
* src/load.c (unload_func_t): Declare a new type for unload.
(struct load_list): Remember the unload symbol if it exists.
(load_object): Move the parsing of the object name from load_file.
Check for the _gmk_unload symbol and if found, remember it.
(load_file): Allow load_object to do object filename parsing.
(unload_file): Remove the load_list entry when unloading the object.
(unload_all): Unload all the loaded objects.
* tests/scripts/features/loadapi: Test the unload function.

NEWS

@@ -29,6 +29,12 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se
   your initialization function can check the provided ABI version to verify
   it's being loaded correctly.
 
+* New feature: Unload function for loaded objects
+  When a loaded object needs to be unloaded by GNU Make, it will invoke an
+  unload function (if one is defined) beforehand that allows the object to
+  perform cleanup operations.
+  Original idea and implementation: Dmitry Goncharov <dgoncharov@users.sf.net>
+
 * New feature: Makefile warning reporting control
   A new option "--warn" controls reporting of warnings for makefiles.  Actions
   can be set to "ignore", "warn", or "error".  Two new warnings are reported:
@@ -38,7 +44,7 @@ https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=111&se
   deprecated, and is translated to "--warn=undefined-vars" internally.
 
 * New feature: Control warnings with the .WARNINGS variable
-  In addition to --warn from the command line, which take effect for make
+  In addition to --warn from the command line, which takes effect for make
   invoked recursively, warnings can be controlled only for the current
   instance of make using the .WARNINGS variable.
 

doc/make.texi

@@ -11990,7 +11990,7 @@ If the load succeeds @code{make} will invoke an initializing function.  If
 @var{symbol-name} is provided, it will be used as the name of the initializing
 function.
 
-If no @var{symbol-name} is provided, the initializing function name is created
+If @var{symbol-name} is not provided, the initializing function name is created
 by taking the base file name of @var{object-file}, up to the first character
 which is not a valid symbol name character (alphanumerics and underscores are
 valid symbol name characters).  To this prefix will be appended the suffix
@@ -12030,6 +12030,31 @@ an error, you can use the @code{-load} directive instead of @code{load}.  GNU
 to load.  The failed object is not added to the @code{.LOADED} variable, which
 can then be consulted to determine if the load was successful.
 
+@subsubheading Unloading Objects
+@cindex unloading objects
+@cindex unload function for loaded objects
+
+When GNU Make needs to unload a loaded object, either because it is exiting or
+because the loaded object has been rebuilt, it will invoke an unload function.
+The unload function name is created by taking the base file name of the object
+file, up to the first character which is not a valid symbol name character
+(alphanumerics and underscores are valid symbol name characters), then
+appending the suffix @code{_gmk_unload}.
+
+If that function exists it will be called using the signature:
+
+@example
+void <name>_gmk_unload (void);
+@end example
+
+If the function does not exist, it will not be called.
+
+Note that only one unload function may be defined per loaded object,
+regardless of how many different setup methods are provided in that loaded
+object.  If your loaded object provides multiple setup methods that require
+unload support it's up to you to coordinate which setups have been invoked in
+the unload function.
+
 @node Initializing Functions, Remaking Loaded Objects, load Directive, Loading Objects
 @subsection Initializing Functions
 @cindex loaded object initializing function
@@ -12275,8 +12300,16 @@ take a prefix as an argument.  First we can write the function in a file
 
 int plugin_is_GPL_compatible;
 
-char *
-gen_tmpfile(const char *nm, int argc, char **argv)
+struct tmpfile @{
+  struct tmpfile *next;
+  char *name;
+@};
+static struct tmpfile *files = NULL;
+@end group
+
+@group
+static char *
+gen_tmpfile(const char *nm, unsigned int argc, char **argv)
 @{
   int fd;
 
@@ -12290,6 +12323,11 @@ gen_tmpfile(const char *nm, int argc, char **argv)
   fd = mkstemp(buf);
   if (fd >= 0)
     @{
+      struct tmpfile *new = malloc (sizeof (struct tmpfile));
+      new->name = strdup (buf);
+      new->next = files;
+      files = new;
+
       /* Don't leak the file descriptor.  */
       close (fd);
       return buf;
@@ -12300,7 +12338,9 @@ gen_tmpfile(const char *nm, int argc, char **argv)
   gmk_free (buf);
   return NULL;
 @}
+@end group
 
+@group
 int
 mk_temp_gmk_setup (unsigned int abi, const gmk_floc *floc)
 @{
@@ -12311,6 +12351,23 @@ mk_temp_gmk_setup (unsigned int abi, const gmk_floc *floc)
   return 1;
 @}
 @end group
+
+@group
+void
+mk_temp_gmk_close ()
+@{
+  while (files)
+    @{
+      struct tmpfile *f = files;
+      files = f->next;
+      printf ("mk_temp removing %s\n", f->name);
+      remove (f->name);
+      free (f->name);
+      free (f);
+    @}
+  printf ("mk_temp plugin closed\n");
+@}
+@end group
 @end example
 
 Next, we will write a @file{Makefile} that can build this shared object, load
@@ -12320,8 +12377,9 @@ it, and use it:
 @group
 all:
         @@echo Temporary file: $(mk-temp tmpfile.)
+        @@echo Temporary file: $(mk-temp tmpfile.)
 
-load mk_temp.so
+-load mk_temp.so
 
 mk_temp.so: mk_temp.c
         $(CC) -shared -fPIC -o $@@ $<
@@ -12332,7 +12390,7 @@ On MS-Windows, due to peculiarities of how shared objects are produced, the
 compiler needs to scan the @dfn{import library} produced when building
 @code{make}, typically called @file{libgnumake-@var{version}.dll.a}, where
 @var{version} is the version of the load object API.  So the recipe to produce
-a shared object will look on Windows like this (assuming the API version is
+a shared object will look like this on Windows (assuming the API version is
 1):
 
 @example
@@ -12345,10 +12403,16 @@ mk_temp.dll: mk_temp.c
 Now when you run @code{make} you'll see something like:
 
 @example
+@group
 $ make
-mk_temp abi 1 plugin loaded from Makefile:4
 cc -shared -fPIC -o mk_temp.so mk_temp.c
-Temporary filename: tmpfile.A7JEwd
+mk_temp abi 1 plugin loaded from Makefile:5
+Temporary file: tmpfile.OYkGMT
+Temporary file: tmpfile.sYsJO0
+mk_temp removing tmpfile.sYsJO0
+mk_temp removing tmpfile.OYkGMT
+mk_temp plugin closed
+@end group
 @end example
 
 @node Integrating make, Features, Extending make, Top

src/gnumake.h

@@ -35,7 +35,16 @@ typedef char *(*gmk_func_ptr)(const char *nm, unsigned int argc, char **argv);
 
      int <setup_fn> (unsigned int abi_version, const gmk_floc *flocp);
 
-   The abi_version will be set to GMK_ABI_VERSION.  */
+   The abi_version will be set to GMK_ABI_VERSION.
+
+   When an object is unloaded by GNU Make, an unload method will be invoked.
+   The name of the method is derived from the filename of the object, with
+   _gmk_unload appended.  It has the signature:
+
+     void <object>_gmk_unload (void);
+
+  There will only be one unload method invoked regardless of the number of
+  setup methods within the object.  */
 
 #ifdef _WIN32
 # ifdef GMK_BUILDING_MAKE

src/load.c

@@ -24,8 +24,6 @@ this program.  If not, see <https://www.gnu.org/licenses/>.  */
 #include <dlfcn.h>
 #include <errno.h>
 
-#define SYMBOL_EXTENSION        "_gmk_setup"
-
 #include "debug.h"
 #include "filedef.h"
 #include "variable.h"
@@ -35,22 +33,32 @@ this program.  If not, see <https://www.gnu.org/licenses/>.  */
 # define RTLD_GLOBAL 0
 #endif
 
+#define GMK_SETUP       "_gmk_setup"
+#define GMK_UNLOAD      "_gmk_unload"
+
+typedef int (*setup_func_t)(unsigned int abi, const floc *flocp);
+typedef void (*unload_func_t)(void);
+
 struct load_list
   {
     struct load_list *next;
     const char *name;
     void *dlp;
+    unload_func_t unload;
   };
 
 static struct load_list *loaded_syms = NULL;
 
-typedef int (*setup_func_t)(unsigned int abi, const floc *flocp);
-
 static setup_func_t
 load_object (const floc *flocp, int noerror, const char *ldname,
-             const char *symname)
+             const char *setupnm)
 {
   static void *global_dl = NULL;
+  char *buf;
+  const char *fp;
+  char *endp;
+  void *dlp;
+  struct load_list *new;
   setup_func_t symp;
 
   if (! global_dl)
@@ -63,62 +71,97 @@ load_object (const floc *flocp, int noerror, const char *ldname,
         }
     }
 
-  symp = (setup_func_t) dlsym (global_dl, symname);
+  /* Find the prefix of the ldname.  */
+  fp = strrchr (ldname, '/');
+#ifdef HAVE_DOS_PATHS
+  if (fp)
+    {
+      const char *fp2 = strchr (fp, '\\');
+
+      if (fp2 > fp)
+        fp = fp2;
+    }
+  else
+    fp = strrchr (ldname, '\\');
+  /* The (improbable) case of d:foo.  */
+  if (fp && *fp && fp[1] == ':')
+    fp++;
+#endif
+  if (!fp)
+    fp = ldname;
+  else
+    ++fp;
+
+  endp = buf = alloca (strlen (fp) + CSTRLEN (GMK_UNLOAD) + 1);
+  while (isalnum ((unsigned char) *fp) || *fp == '_')
+    *(endp++) = *(fp++);
+
+  /* If we didn't find a symbol name yet, construct it from the prefix.  */
+  if (! setupnm)
+    {
+      memcpy (endp, GMK_SETUP, CSTRLEN (GMK_SETUP) + 1);
+      setupnm = buf;
+    }
+
+  DB (DB_VERBOSE, (_("Loading symbol %s from %s\n"), setupnm, ldname));
+
+  symp = (setup_func_t) dlsym (global_dl, setupnm);
+  if (symp)
+    return symp;
+
+  /* If the path has no "/", try the current directory first.  */
+  dlp = NULL;
+  if (! strchr (ldname, '/')
+#ifdef HAVE_DOS_PATHS
+      && ! strchr (ldname, '\\')
+#endif
+      )
+    dlp = dlopen (concat (2, "./", ldname), RTLD_LAZY|RTLD_GLOBAL);
+
+  /* If we haven't opened it yet, try the default search path.  */
+  if (! dlp)
+    dlp = dlopen (ldname, RTLD_LAZY|RTLD_GLOBAL);
+
+  /* Still no?  Then fail.  */
+  if (! dlp)
+    {
+      const char *err = dlerror ();
+      if (noerror)
+        DB (DB_BASIC, ("%s\n", err));
+      else
+        OS (error, flocp, "%s", err);
+      return NULL;
+    }
+
+  DB (DB_VERBOSE, (_("Loaded shared object %s\n"), ldname));
+
+  /* Assert that the GPL license symbol is defined.  */
+  symp = (setup_func_t) dlsym (dlp, "plugin_is_GPL_compatible");
+  if (! symp)
+    OS (fatal, flocp,
+        _("loaded object %s is not declared to be GPL compatible"), ldname);
+
+  symp = (setup_func_t) dlsym (dlp, setupnm);
   if (! symp)
     {
-      struct load_list *new;
-      void *dlp = NULL;
-
-    /* If the path has no "/", try the current directory first.  */
-      if (! strchr (ldname, '/')
-#ifdef HAVE_DOS_PATHS
-          && ! strchr (ldname, '\\')
-#endif
-         )
-        dlp = dlopen (concat (2, "./", ldname), RTLD_LAZY|RTLD_GLOBAL);
-
-      /* If we haven't opened it yet, try the default search path.  */
-      if (! dlp)
-        dlp = dlopen (ldname, RTLD_LAZY|RTLD_GLOBAL);
-
-      /* Still no?  Then fail.  */
-      if (! dlp)
-        {
-          const char *err = dlerror ();
-          if (noerror)
-            DB (DB_BASIC, ("%s\n", err));
-          else
-            OS (error, flocp, "%s", err);
-          return NULL;
-        }
-
-      DB (DB_VERBOSE, (_("Loaded shared object %s\n"), ldname));
-
-      /* Assert that the GPL license symbol is defined.  */
-      symp = (setup_func_t) dlsym (dlp, "plugin_is_GPL_compatible");
-      if (! symp)
-        OS (fatal, flocp,
-             _("loaded object %s is not declared to be GPL compatible"),
-             ldname);
-
-      symp = (setup_func_t) dlsym (dlp, symname);
-      if (! symp)
-        {
-          const char *err = dlerror ();
-          OSSS (fatal, flocp, _("failed to load symbol %s from %s: %s"),
-                symname, ldname, err);
-        }
-
-      /* Add this symbol to a trivial lookup table.  This is not efficient but
-         it's highly unlikely we'll be loading lots of objects, and we only
-         need it to look them up on unload, if we rebuild them.  */
-      new = xmalloc (sizeof (struct load_list));
-      new->name = xstrdup (ldname);
-      new->dlp = dlp;
-      new->next = loaded_syms;
-      loaded_syms = new;
+      const char *err = dlerror ();
+      OSSS (fatal, flocp, _("failed to load symbol %s from %s: %s"),
+            setupnm, ldname, err);
     }
 
+  new = xcalloc (sizeof (struct load_list));
+  new->next = loaded_syms;
+  loaded_syms = new;
+  new->name = ldname;
+  new->dlp = dlp;
+
+  /* Compute the name of the unload function and look it up.  */
+  memcpy (endp, GMK_UNLOAD, CSTRLEN (GMK_UNLOAD) + 1);
+
+  new->unload = (unload_func_t) dlsym (dlp, buf);
+  if (new->unload)
+    DB (DB_VERBOSE, (_("Detected symbol %s in %s\n"), buf, ldname));
+
   return symp;
 }
 
@@ -126,9 +169,8 @@ int
 load_file (const floc *flocp, struct file *file, int noerror)
 {
   const char *ldname = file->name;
-  size_t nmlen = strlen (ldname);
-  char *new = alloca (nmlen + CSTRLEN (SYMBOL_EXTENSION) + 1);
-  char *symname = NULL;
+  char *buf;
+  char *setupnm = NULL;
   const char *fp;
   int r;
   setup_func_t symp;
@@ -153,15 +195,15 @@ load_file (const floc *flocp, struct file *file, int noerror)
             OS (fatal, flocp, _("empty symbol name for load: %s"), ldname);
 
           /* Make a copy of the ldname part.  */
-          memcpy (new, ldname, l);
-          new[l] = '\0';
-          ldname = new;
-          nmlen = l;
+          buf = alloca (strlen (ldname) + 1);
+          memcpy (buf, ldname, l);
+          buf[l] = '\0';
+          ldname = buf;
 
           /* Make a copy of the symbol name part.  */
-          symname = new + l + 1;
-          memcpy (symname, fp, ep - fp);
-          symname[ep - fp] = '\0';
+          setupnm = buf + l + 1;
+          memcpy (setupnm, fp, ep - fp);
+          setupnm[ep - fp] = '\0';
         }
     }
 
@@ -175,40 +217,8 @@ load_file (const floc *flocp, struct file *file, int noerror)
   if (file && file->loaded)
     return -1;
 
-  /* If we didn't find a symbol name yet, construct it from the ldname.  */
-  if (! symname)
-    {
-      char *p = new;
-
-      fp = strrchr (ldname, '/');
-#ifdef HAVE_DOS_PATHS
-      if (fp)
-        {
-          const char *fp2 = strchr (fp, '\\');
-
-          if (fp2 > fp)
-            fp = fp2;
-        }
-      else
-        fp = strrchr (ldname, '\\');
-      /* The (improbable) case of d:foo.  */
-      if (fp && *fp && fp[1] == ':')
-        fp++;
-#endif
-      if (!fp)
-        fp = ldname;
-      else
-        ++fp;
-      while (isalnum ((unsigned char) *fp) || *fp == '_')
-        *(p++) = *(fp++);
-      strcpy (p, SYMBOL_EXTENSION);
-      symname = new;
-    }
-
-  DB (DB_VERBOSE, (_("Loading symbol %s from %s\n"), symname, ldname));
-
   /* Load it!  */
-  symp = load_object (flocp, noerror, ldname, symname);
+  symp = load_object (flocp, noerror, ldname, setupnm);
   if (! symp)
     return 0;
 
@@ -228,22 +238,53 @@ load_file (const floc *flocp, struct file *file, int noerror)
 int
 unload_file (const char *name)
 {
-  int rc = 0;
-  struct load_list *d;
+  struct load_list **dp = &loaded_syms;
 
-  for (d = loaded_syms; d != NULL; d = d->next)
-    if (streq (d->name, name) && d->dlp)
-      {
-        DB (DB_VERBOSE, (_("Unloading shared object %s\n"), name));
-        rc = dlclose (d->dlp);
-        if (rc)
-          perror_with_name ("dlclose: ", d->name);
-        else
-          d->dlp = NULL;
-        break;
-      }
+  /* Unload and remove the entry for this file.  */
+  while (*dp != NULL)
+    {
+      struct load_list *d = *dp;
 
-  return rc;
+      if (streq (d->name, name))
+        {
+          int rc;
+
+          DB (DB_VERBOSE, (_("Unloading shared object %s\n"), name));
+
+          if (d->unload)
+            (*d->unload) ();
+
+          rc = dlclose (d->dlp);
+          if (rc)
+            perror_with_name ("dlclose: ", d->name);
+
+          *dp = d->next;
+          free (d);
+          return rc;
+        }
+
+      dp = &d->next;
+    }
+
+  return 0;
+}
+
+void
+unload_all ()
+{
+  while (loaded_syms)
+    {
+      struct load_list *d = loaded_syms;
+      loaded_syms = loaded_syms->next;
+
+      if (d->unload)
+        (*d->unload) ();
+
+      if (dlclose (d->dlp))
+        perror_with_name ("dlclose: ", d->name);
+
+      free (d);
+    }
 }
 
 #else
@@ -264,4 +305,9 @@ unload_file (const char *name UNUSED)
   O (fatal, NILF, "INTERNAL: cannot unload when load is not supported");
 }
 
+void
+unload_all ()
+{
+}
+
 #endif  /* MAKE_LOAD */

src/main.c

@@ -3815,6 +3815,10 @@ die (int status)
       if (verify_flag)
         verify_file_data_base ();
 
+      /* Unload plugins before jobserver integrity check in case a plugin
+       * participates in jobserver.  */
+      unload_all ();
+
       clean_jobserver (status);
 
       if (output_context)

src/makeint.h

@@ -674,6 +674,7 @@ int guile_gmake_setup (const floc *flocp);
 /* Loadable object support.  Sets to the strcached name of the loaded file.  */
 int load_file (const floc *flocp, struct file *file, int noerror);
 int unload_file (const char *name);
+void unload_all (void);
 
 /* Maintainer mode support */
 #ifdef MAKE_MAINTAINER_MODE

tests/scripts/features/loadapi

@@ -93,6 +93,32 @@ testapi_gmk_setup (unsigned int abi, const gmk_floc *floc)
 
     return 1;
 }
+
+int
+alternative_setup ()
+{
+    gmk_add_function ("test-expand", func_test, 1, 1, GMK_FUNC_DEFAULT);
+    gmk_add_function ("test-noexpand", func_test, 1, 1, GMK_FUNC_NOEXPAND);
+    gmk_add_function ("test-eval", func_test, 1, 1, GMK_FUNC_DEFAULT);
+    gmk_add_function ("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.", func_test, 0, 0, 0);
+
+    if (getenv ("TESTAPI_VERBOSE"))
+      printf ("alternative_setup\n");
+
+    if (getenv ("TESTAPI_KEEP"))
+      return -1;
+
+    return 1;
+}
+
+void
+testapi_gmk_unload ()
+{
+    const char *s = getenv ("TESTAPI_VERBOSE");
+    if (s && *s == '3')
+      printf ("testapi_gmk_unload\n");
+}
+
 EOF
 close($F) or die "close: testapi.c: $!\n";
 
@@ -222,7 +248,70 @@ force:;
 ", '', "testapi_gmk_setup\n#MAKEFILE#:2\ntestapi.so\ntestapi_gmk_setup\n#MAKEFILE#:2\nhello\n#MAKE#: 'all' is up to date.\n");
 }
 
+my @names  = ('testapi.so', './testapi.so', '#PWD#/testapi.so');
+
+for my $name (@names) {
+
+# Test the make correctly figures out the name of the close function and runs
+# the close function.
+$ENV{TESTAPI_VERBOSE} = 3;
+run_make_test("
+load $name
+all:; \$(info \$(test-expand hello))
+", '', "testapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload\n");
+}
+
+# Same as above, but the setup function is custom.
+@names  = ('testapi.so(alternative_setup)', './testapi.so(alternative_setup)',
+           '#PWD#/testapi.so(alternative_setup)');
+for my $name (@names) {
+
+# Test the close function.
+$ENV{TESTAPI_VERBOSE} = 3;
+run_make_test("
+load $name
+all:; \$(info \$(test-expand hello))
+", '', "alternative_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload\n");
+}
+
+# Test that makes runs the close function on failure.
+$ENV{TESTAPI_VERBOSE} = 3;
+run_make_test(q!
+load testapi.so
+all: bad_preqreq; :
+!, '', "testapi_gmk_setup\n#MAKE#: *** No rule to make target 'bad_preqreq', needed by 'all'.  Stop.\ntestapi_gmk_unload\n", 512);
+
+# Test that make unloads a shared object, calls the close function, loads
+# the plugin again, and then calls the close function again on exit.
+&utouch(-10, 'testapi.so');
+$ENV{TESTAPI_VERBOSE} = 3;
+run_make_test("
+load testapi.so
+all:; \$(info \$(test-expand hello))
+testapi.so: testapi.c; $sobuild
+", '', "testapi_gmk_setup\ntestapi_gmk_unload\n$sobuild\ntestapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload");
+
+# Test that make unloads a shared object, calls the close function, loads
+# the plugin again, and then calls the close function again on exit.
+$ENV{TESTAPI_VERBOSE} = 3;
+run_make_test(q!
+load testapi.so
+all:; $(info $(test-expand hello))
+testapi.so: force; $(info $@)
+force:;
+.PHONY: force
+!, '', "testapi_gmk_setup\ntestapi_gmk_unload\ntestapi.so\ntestapi_gmk_setup\nhello\n#MAKE#: 'all' is up to date.\ntestapi_gmk_unload\n");
+
 unlink(qw(testapi.c testapi.so)) unless $keep;
 
+# Test that make does not run the close function, unless the shared object
+# loaded successfully.
+unlink('testapi.so');
+$ENV{TESTAPI_VERBOSE} = 3;
+run_make_test(q!
+load testapi.so
+all:; :
+!, '', "#MAKEFILE#:2: testapi.so: cannot open shared object file: $ERR_no_such_file\n#MAKEFILE#:2: *** testapi.so: failed to load.  Stop.\n", 512);
+
 # This tells the test driver that the perl test script executed properly.
 1;