octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: some notes about mex support in Octave


From: John W. Eaton
Subject: Re: some notes about mex support in Octave
Date: Wed, 26 Jul 2006 15:27:38 -0400

On 26-Jul-2006, David Bateman wrote:

| John W. Eaton wrote:
| 
| >On 26-Jul-2006, David Bateman wrote:
| >
| >| Ok, then what about the attached patch.
| >
| >Thanks, I checked it in.
| >
| >| The test code should probably be
| >| cleaned up. Also I note that valgrind is not clean against the mex
| >| types, though that is independent of the sparse type..
| >
| >Can you send an example of some code that shows the problem?
| >  
| >
| Well in fact I had problems with all of the mex example code in
| valgrind. For example launching octave with "valgrind --tool-memcheck
| octave" and then running the mystruct mex example gave me the errors
| 
| octave:1> a.b=1; a.c=2; mystruct(a)

OK, I think the following changes fix this problem.

Thanks,

jwe


src/ChangeLog:

2006-07-26  John W. Eaton  <address@hidden>

        * mex.cc (xfree): New function.
        (mex::free): Use it.
        (mxArray_struct::~mxArray_struct, mxArray_cell::~mxArray_cell):
        Delete elements with delete, not mxDestroyArray.
        (mex::cleanup): Don't call mex::free or mex::free_value.
        (mex::free_value): Add debug warning.
        (mex::mark, mex::unmark): Fix debug warning.
        (call_mex): Use unwind_protect frame.
        (mexUnlock): Use iterator to remove item from mex_lock_count.


Index: src/mex.cc
===================================================================
RCS file: /cvs/octave/src/mex.cc,v
retrieving revision 1.6
diff -u -u -r1.6 mex.cc
--- src/mex.cc  26 Jul 2006 03:36:33 -0000      1.6
+++ src/mex.cc  26 Jul 2006 19:26:22 -0000
@@ -29,6 +29,16 @@
 
 // #define DEBUG 1
 
+static void
+xfree (void *ptr)
+{
+#ifdef DEBUG
+  std::cerr << "free: " << ptr << std::endl;
+#endif
+
+  ::free (ptr);
+}
+
 static int
 max_str_len (int m, const char **str)
 {
@@ -1518,7 +1528,7 @@
     int ntot = nfields * get_number_of_elements ();
 
     for  (int i = 0; i < ntot; i++)
-      mxDestroyArray (data[i]);
+      delete data[i];
 
     mxFree (data);
   }
@@ -1738,7 +1748,7 @@
     int nel = get_number_of_elements ();
 
     for  (int i = 0; i < nel; i++)
-      mxDestroyArray (data[i]);
+      delete data[i];
 
     mxFree (data);
   }
@@ -1887,7 +1897,7 @@
   ~mex (void)
   {
     if (! memlist.empty ())
-      error ("mex: cleanup failed");
+      error ("mex: %s: cleanup failed", function_name ());
 
     mxFree (fname);
   }
@@ -1915,13 +1925,26 @@
   {
     mex *context = static_cast<mex *> (ptr);
 
+    // We can't use mex::free here because it modifies memlist.
     for (std::set<void *>::iterator p = context->memlist.begin ();
         p != context->memlist.end (); p++)
-      context->free (*p);
+      {
+       if (*p)
+         {
+           context->unmark (*p);
+
+           xfree (*p);
+         }
+      }
+
+    context->memlist.clear ();
 
+    // We can't use mex::free_value here because it modifies arraylist.
     for (std::set<mxArray *>::iterator p = context->arraylist.begin ();
         p != context->arraylist.end (); p++)
-      context->free_value (*p);
+      delete *p;
+
+    context->arraylist.clear ();
   }
 
   // allocate a pointer, and mark it to be freed on exit
@@ -1938,7 +1961,7 @@
        // FIXME -- could use "octave_new_handler();" instead
 
        error ("%s: failed to allocate %d bytes of memory",
-              mexFunctionName (), n);
+              function_name (), n);
 
        abort ();
       }
@@ -2018,10 +2041,7 @@
          {
            global_memlist.erase (p);
 
-#ifdef DEBUG
-           std::cerr << "free: " << ptr << std::endl;
-#endif
-           ::free (ptr);
+           xfree (ptr);
          }
        else
          warning ("mxFree: skipping memory not allocated by mxMalloc, 
mxCalloc, or mxRealloc");
@@ -2043,8 +2063,17 @@
   // Free an array and its contents.
   void free_value (mxArray *ptr)
   {
-    arraylist.erase (ptr);
-    delete ptr;
+    std::set<mxArray *>::iterator p = arraylist.find (ptr);
+
+    if (p != arraylist.end ())
+      {
+       arraylist.erase (p);
+       delete ptr;
+      }
+#ifdef DEBUG
+    else
+      warning ("mex::free_value: skipping memory not allocated by 
mex::make_value");
+#endif
   }
 
   // Mark an array and its contents so it will not be freed on exit.
@@ -2077,7 +2106,7 @@
   {
 #ifdef DEBUG
     if (memlist.find (p) != memlist.end ())
-      warning ("%s: double registration ignored", mexFunctionName ());
+      warning ("%s: double registration ignored", function_name ());
 #endif
 
     memlist.insert (p);
@@ -2085,39 +2114,44 @@
 
   // Unmark a pointer to be freed on exit, either because it was
   // made persistent, or because it was already freed.
-  void unmark (void *p)
+  void unmark (void *ptr)
   {
+    std::set<void *>::iterator p = memlist.find (ptr);
+
+    if (p != memlist.end ())
+      memlist.erase (p);
 #ifdef DEBUG
-    if (memlist.find (p) != memlist.end ())
-      warning ("%s: value not marked", mexFunctionName ());
+    else
+      warning ("%s: value not marked", function_name ());
 #endif
-
-    memlist.erase (p);
   }
 
   // List of memory resources we allocated.
   static std::set<void *> global_memlist;
 
   // Mark a pointer as one we allocated.
-  void global_mark (void *p)
+  void global_mark (void *ptr)
   {
 #ifdef DEBUG
-    if (global_memlist.find (p) != global_memlist.end ())
-      warning ("%s: double registration ignored", mexFunctionName ());
+    if (global_memlist.find (ptr) != global_memlist.end ())
+      warning ("%s: double registration ignored", function_name ());
 #endif
 
-    global_memlist.insert (p);
+    global_memlist.insert (ptr);
   }
 
   // Unmark a pointer as one we allocated.
-  void global_unmark (void *p)
+  void global_unmark (void *ptr)
   {
+    std::set<void *>::iterator p = global_memlist.find (ptr);
+
+    if (p != global_memlist.end ())
+      global_memlist.erase (p);
 #ifdef DEBUG
-    if (global_memlist.find (p) != global_memlist.end ())
-      warning ("%s: value not marked", mexFunctionName ());
+    else
+      warning ("%s: value not marked", function_name ());
 #endif
 
-    global_memlist.erase (p);
   }
 };
 
@@ -2773,6 +2807,11 @@
   for (int i = 0; i < nout; i++)
     argout[i] = 0;
 
+  unwind_protect::begin_frame ("call_mex");
+
+  // Save old mex pointer.
+  unwind_protect_ptr (mex_context);
+
   mex context;
 
   unwind_protect::add (mex::cleanup, static_cast<void *> (&context));
@@ -2780,9 +2819,6 @@
   for (int i = 0; i < nargin; i++)
     argin[i] = context.make_value (args(i));
 
-  // Save old mex pointer.
-  unwind_protect_ptr (mex_context);
-
   if (setjmp (context.jump) == 0)
     {
       mex_context = &context;
@@ -2804,9 +2840,6 @@
        }
     }
 
-  // Restore old mex pointer.
-  unwind_protect::run ();
-
   // Convert returned array entries back into octave values.
 
   octave_value_list retval;
@@ -2832,7 +2865,7 @@
     }
 
   // Clean up mex resources.
-  unwind_protect::run ();
+  unwind_protect::run_frame ("call_mex");
 
   return retval;
 }
@@ -3156,7 +3189,9 @@
     {
       const char *fname = mexFunctionName ();
 
-      if (mex_lock_count.find (fname) == mex_lock_count.end ())
+      std::map<std::string,int>::iterator p = mex_lock_count.find (fname);
+
+      if (p == mex_lock_count.end ())
        warning ("mexUnlock: funtion `%s' is not locked", fname);
       else
        {
@@ -3166,7 +3201,7 @@
            {
              munlock (fname);
 
-             mex_lock_count.erase (fname);
+             mex_lock_count.erase (p);
            }
        }
     }


reply via email to

[Prev in Thread] Current Thread [Next in Thread]