changeset 7749:14e05160b99f

reference counting for functions loaded from shared libraries
author John W. Eaton <jwe@octave.org>
date Thu, 01 May 2008 17:25:56 -0400
parents 3dc91baee089
children 5c6c6f4803c8
files liboctave/ChangeLog liboctave/oct-shlib.cc liboctave/oct-shlib.h src/ChangeLog src/dynamic-ld.cc src/parse.y
diffstat 6 files changed, 96 insertions(+), 107 deletions(-) [+]
line wrap: on
line diff
--- a/liboctave/ChangeLog	Thu May 01 14:32:52 2008 -0400
+++ b/liboctave/ChangeLog	Thu May 01 17:25:56 2008 -0400
@@ -1,3 +1,16 @@
+2008-05-01  John W. Eaton  <jwe@octave.org>
+
+	* oct-shlib.h (octave_shlib::number_of_functions_loaded):
+	Return size_t instead of int value.
+	* oct-shlib.cc (octave_base_shlib::number_of_functions_loaded):
+	Likewise.
+	(octave_base_shlib::fcn_names): Now a std::map object.
+	Adjust all uses.
+	(octave_base_shlib::fcn_names_iterator,
+	octave_base_shlib::fcn_names_const_iterator): New typedefs.
+	(octave_base_shlib::add_to_fcn_names, octave_base_shlib::remove):
+	Perform reference counting for functions accessed.
+
 2008-04-30  Jaroslav Hajek <highegg@gmail.com>
 
 	* lo-mappers.cc (xlog2 (double)): Compute log (2), not log2 (2).
--- a/liboctave/oct-shlib.cc	Thu May 01 14:32:52 2008 -0400
+++ b/liboctave/oct-shlib.cc	Thu May 01 17:25:56 2008 -0400
@@ -24,6 +24,8 @@
 #include <config.h>
 #endif
 
+#include <map>
+
 #if defined (HAVE_SHL_LOAD_API)
 #include <cerrno>
 #include <cstring>
@@ -77,6 +79,14 @@
 
   void open (const std::string&) { }
 
+  // FIXME -- instead of returning a direct pointer to a function,
+  // perhaps this should return an object containing the pointer.
+  // Then we could do the reference counting without relying on users
+  // of this class to call remove at the appropriate time.  However,
+  // when users accessed the internal pointer, they would still need
+  // to know that the pointer is only valid while the container is
+  // valid (similar to the std::string::c_str method).
+
   void *search (const std::string&, name_mangler = 0) { return 0; }
 
   void close (octave_shlib::close_hook = 0) { }
@@ -87,7 +97,7 @@
 
   bool is_out_of_date (void) const;
 
-  int number_of_functions_loaded (void) const { return fcn_names.length (); }
+  size_t number_of_functions_loaded (void) const { return fcn_names.size (); }
 
   std::string file_name (void) const { return file; }
 
@@ -95,9 +105,12 @@
 
 protected:
 
+  typedef std::map<std::string, size_t>::iterator fcn_names_iterator;
+  typedef std::map<std::string, size_t>::const_iterator fcn_names_const_iterator;
+
   std::string file;
 
-  string_vector fcn_names;
+  std::map<std::string, size_t> fcn_names;
 
   octave_time tm_loaded;
 
@@ -121,24 +134,14 @@
 {
   bool retval = false;
 
-  int n = number_of_functions_loaded ();
-
-  string_vector new_fcn_names (n);
+  fcn_names_iterator p = fcn_names.find (fcn_name);
 
-  int k = 0;
-
-  for (int i = 0; i < n; i++)
+  if (--(p->second) == 0)
     {
-      if (fcn_names(i) == fcn_name)
-	retval = true;
-      else
-	new_fcn_names(k++) = fcn_names(i);
+      fcn_names.erase (fcn_name);
+      retval = true;
     }
 
-  new_fcn_names.resize (k);
-
-  fcn_names = new_fcn_names;
-
   return retval;
 }
 
@@ -166,24 +169,19 @@
 void
 octave_base_shlib::add_to_fcn_names (const std::string& name)
 {
-  int n = number_of_functions_loaded ();
+  fcn_names_iterator p = fcn_names.find (name);
 
-  for (int i = 0; i < n; i++)
-    if (fcn_names(i) == name)
-      return;
-
-  fcn_names.resize (n+1);
-
-  fcn_names(n) = name;
+  if (p == fcn_names.end ())
+    fcn_names[name] = 1;
+  else
+    ++(p->second);
 }
 
 void
 octave_base_shlib::do_close_hook (octave_shlib::close_hook cl_hook)
 {
-  int n = number_of_functions_loaded ();
-
-  for (int i = 0; i < n; i++)
-    cl_hook (fcn_names(i));
+  for (fcn_names_iterator p = fcn_names.begin (); p != fcn_names.end (); p++)
+    cl_hook (p->first);
 }
 
 void
@@ -191,7 +189,7 @@
 {
   file = "";
 
-  fcn_names.resize (0);
+  fcn_names.clear ();
 
   tm_loaded = static_cast<time_t> (0);
 }
--- a/liboctave/oct-shlib.h	Thu May 01 14:32:52 2008 -0400
+++ b/liboctave/oct-shlib.h	Thu May 01 17:25:56 2008 -0400
@@ -89,7 +89,7 @@
   operator bool () const { return is_open (); }
 
   virtual void open (const std::string& f) { rep->open (f); }
-  
+
   virtual void *search (const std::string& nm, name_mangler mangler = 0)
     { return rep->search (nm, mangler); }
 
@@ -106,7 +106,7 @@
 
   bool is_relative (void) const { return relative; }
 
-  virtual int number_of_functions_loaded (void) const
+  virtual size_t number_of_functions_loaded (void) const
     { return rep->number_of_functions_loaded (); }
 
   virtual std::string file_name (void) const
--- a/src/ChangeLog	Thu May 01 14:32:52 2008 -0400
+++ b/src/ChangeLog	Thu May 01 17:25:56 2008 -0400
@@ -1,5 +1,13 @@
 2008-05-01  John W. Eaton  <jwe@octave.org>
 
+	* parse.y (load_fcn_from_file): Expect
+	* dynamic-ld.cc (octave_dynamic_loader::do_load_oct):
+	Search currently loaded .oct files by file name.  Don't search
+	currently loaded files for functions.
+	(octave_shlib_list::find_file, octave_shlib_list::do_find_file):
+	New functions.
+	(octave_shlib_list::search, octave_shlib_list::do_search): Delete.
+
 	* dynamic-ld.cc (octave_dynamic_loader::do_load_oct): No need to
 	do load_path lookups here.
 	(octave_shlib_list::iterator, octave_shlib_list::const_iterator):
--- a/src/dynamic-ld.cc	Thu May 01 14:32:52 2008 -0400
+++ b/src/dynamic-ld.cc	Thu May 01 17:25:56 2008 -0400
@@ -58,8 +58,7 @@
 
   static void remove (octave_shlib& shl, octave_shlib::close_hook cl_hook = 0);
 
-  static void *search (const std::string& fcn_name, octave_shlib& shl,
-		       octave_shlib::name_mangler mangler = 0);
+  static octave_shlib find_file (const std::string& file_name);
 
   static void display (void);
 
@@ -73,8 +72,7 @@
 
   void do_remove (octave_shlib& shl, octave_shlib::close_hook cl_hook = 0);
 
-  void *do_search (const std::string& fcn_name, octave_shlib& shl,
-		   octave_shlib::name_mangler mangler = 0);
+  octave_shlib do_find_file (const std::string& file_name) const;
 
   void do_display (void) const;
 
@@ -117,27 +115,21 @@
     }
 }
 
-void *
-octave_shlib_list::do_search (const std::string& fcn_name, octave_shlib& shl,
-			      octave_shlib::name_mangler mangler)
+octave_shlib
+octave_shlib_list::do_find_file (const std::string& file_name) const
 {
-  void *function = 0;
-
-  shl = octave_shlib ();
+  octave_shlib retval;
 
-  for (iterator p = lib_list.begin (); p != lib_list.end (); p++)
+  for (const_iterator p = lib_list.begin (); p != lib_list.end (); p++)
     {
-      function = p->search (fcn_name, mangler);
-
-      if (function)
+      if (p->file_name () == file_name)
 	{
-	  shl = *p;
-
+	  retval = *p;
 	  break;
 	}
     }
 
-  return function;
+  return retval;
 }
 
 void
@@ -181,11 +173,11 @@
     instance->do_remove (shl, cl_hook);
 }
 
-void *
-octave_shlib_list::search (const std::string& fcn_name, octave_shlib& shl,
-			   octave_shlib::name_mangler mangler)
+octave_shlib
+octave_shlib_list::find_file (const std::string& file_name)
 {
-  return (instance_ok ()) ? instance->do_search (fcn_name, shl, mangler) : 0;
+  return (instance_ok ())
+    ? instance->do_find_file (file_name) : octave_shlib ();
 }
 
 void
@@ -337,70 +329,51 @@
 {
   octave_function *retval = 0;
 
-  octave_shlib oct_file;
-
   unwind_protect::begin_frame ("octave_dynamic_loader::do_load");
 
   unwind_protect_bool (octave_dynamic_loader::doing_load);
 
   doing_load = true;
 
-  void *function
-    = octave_shlib_list::search (fcn_name, oct_file, xmangle_name);
+  octave_shlib oct_file = octave_shlib_list::find_file (file_name);
+
+  if (oct_file && oct_file.is_out_of_date ())
+    clear (oct_file);
+
+  if (! oct_file)
+    {
+      oct_file.open (file_name);
+
+      if (! error_state && oct_file)
+	{
+	  octave_shlib_list::append (oct_file);
+
+	  if (relative)
+	    oct_file.mark_relative ();
+	}
+    }
 
   if (! error_state)
     {
-      bool reloading = false;
-
-      if (function)
-	{
-	  // If there is already a function by this name installed
-	  // from the same file, clear the file so we can reload it.
-
-	  // If there is already a function by this name installed
-	  // from a different file, leave the other file alone and
-	  // load the function from the new file.
-
-	  reloading = same_file (file_name, oct_file.file_name ());
-
-	  if (reloading)
-	    clear (oct_file);
-
-	  function = 0;
-	}
-
-      if (! reloading)
-	oct_file = octave_shlib ();
-
-      oct_file.open (file_name);
-
-      if (! error_state)
+      if (oct_file)
 	{
-	  if (oct_file)
+	  void *function = oct_file.search (fcn_name, xmangle_name);
+
+	  if (function)
 	    {
-	      if (relative)
-		oct_file.mark_relative ();
+	      octave_dld_fcn_getter f
+		= FCN_PTR_CAST (octave_dld_fcn_getter, function);
 
-	      octave_shlib_list::append (oct_file);
+	      retval = f (oct_file, relative);
 
-	      function = oct_file.search (fcn_name, xmangle_name);
+	      if (! retval)
+		::error ("failed to install .oct file function `%s'",
+			 fcn_name.c_str ());
 	    }
-	  else
-	    ::error ("%s is not a valid shared library",
-		     file_name.c_str ());
 	}
-    }
-
-  if (function)
-    {
-      octave_dld_fcn_getter f
-	= FCN_PTR_CAST (octave_dld_fcn_getter, function);
-
-      retval = f (oct_file, relative);
-
-      if (! retval)
-	::error ("failed to install .oct file function `%s'",
-		 fcn_name.c_str ());
+      else
+	::error ("%s is not a valid shared library",
+		 file_name.c_str ());
     }
   
   unwind_protect::run_frame ("octave_dynamic_loader::do_load");
--- a/src/parse.y	Thu May 01 14:32:52 2008 -0400
+++ b/src/parse.y	Thu May 01 17:25:56 2008 -0400
@@ -3225,12 +3225,9 @@
       autoloading = true;
     }
 
-  if (! file.empty ())
-    {
-      fcn_file_from_relative_lookup = ! octave_env::absolute_pathname (file);
-
-      file = octave_env::make_absolute (file, octave_env::getcwd ());
-    }
+  fcn_file_from_relative_lookup = ! octave_env::absolute_pathname (file);
+
+  file = octave_env::make_absolute (file, octave_env::getcwd ());
 
   int len = file.length ();