fix memory leaks

ampl · Oct 18, 2024 · 7f525f3 · 7f525f3
1 parent 7f9a59d
commit 7f525f3
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 16 deletions.
diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml
@@ -69,10 +69,10 @@ jobs:
           CIBW_TEST_COMMAND: ${{ env.CIBW_TEST_COMMAND }}
           CIBW_TEST_REQUIRES: --index-url https://pypi.ampl.com --extra-index-url https://pypi.org/simple ampl_module_base
 
-      - name: Escape build variable
+      - name: Remove '-*' from build variable
         run: |
-          # Replace '*' with '' and store it in an environment variable
-          ESCAPED_BUILD=$(echo "${{ matrix.build }}" | sed 's/\*//g')
+          # Remove '-*' from the build variable
+          ESCAPED_BUILD=$(echo "${{ matrix.build }}" | sed 's/-\*//g')
           echo "ESCAPED_BUILD=$ESCAPED_BUILD" >> $GITHUB_ENV
 
       - uses: actions/upload-artifact@v4

diff --git a/amplpy/ampl.pyx b/amplpy/ampl.pyx
@@ -104,7 +104,7 @@ cdef class AMPL:
     cdef object _output_handler
     cdef object _error_handler
 
-    def __cinit__(self, environment=None):
+    def __init__(self, environment=None):
         """
         Constructor:
         creates a new AMPL instance with the specified environment if provided.

diff --git a/amplpy/entity.pxi b/amplpy/entity.pxi
@@ -66,7 +66,7 @@ cdef class Entity(object):
         if not isinstance(index, (tuple, list)):
             index = [index]
         cdef campl.AMPL_TUPLE* tuple_c =  to_c_tuple(index)
-        return m(self.wrap_function, self._c_ampl, self._name, tuple_c)
+        return create_entity(self.wrap_function, self._c_ampl, self._name, tuple_c)
 
     def get(self, *index):
         """
@@ -82,14 +82,14 @@ cdef class Entity(object):
             index = index[0]
             index = list(index)
         if len(index) == 0:
-            return m(self.wrap_function, self._c_ampl, self._name, NULL)
+            return create_entity(self.wrap_function, self._c_ampl, self._name, NULL)
         else:
             tuple_c =  to_c_tuple(index)
             if self.wrap_function == campl.AMPL_PARAMETER:
                 campl.AMPL_InstanceGetName(self._c_ampl, self._name.encode('utf-8'), tuple_c, &name_c)
-                return m(self.wrap_function, self._c_ampl, name_c.decode('utf-8'), NULL).value()
+                return create_entity(self.wrap_function, self._c_ampl, name_c.decode('utf-8'), NULL).value()
             else:
-                return m(self.wrap_function, self._c_ampl, self._name, tuple_c)
+                return create_entity(self.wrap_function, self._c_ampl, self._name, tuple_c)
 
     def find(self, index):
         """
@@ -106,7 +106,7 @@ cdef class Entity(object):
         for i in range(size):
             if campl.AMPL_TupleCompare(index_c, indices_c[i]) == 0:
                 free(indices_c)
-                return m(self.wrap_function, self._c_ampl, self._name, index_c)
+                return create_entity(self.wrap_function, self._c_ampl, self._name, index_c)
         free(indices_c)
         return None
 

diff --git a/amplpy/iterators.pxi b/amplpy/iterators.pxi
@@ -65,6 +65,12 @@ cdef class EntityMap(object):
         entityit.end = entityit.begin + entityit._size
         return entityit
 
+    def __dealloc__(self):
+        if self.begin != NULL:
+            for i in range(self._size):
+                free(self.begin[i])
+            free(self.begin)
+
     def __iter__(self):
         self.iterator = self.begin
         return self
@@ -75,14 +81,14 @@ cdef class EntityMap(object):
         cdef char** it = self.iterator
         self.iterator += 1
         name = it[0].decode('utf-8')
-        return (name, m(self.entity_class, self._c_ampl, name, NULL))
+        return (name, create_entity(self.entity_class, self._c_ampl, name, NULL))
 
     def __getitem__(self, key):
         assert isinstance(key, str)
         cdef campl.AMPL_ENTITYTYPE entitytype
         PY_AMPL_CALL(campl.AMPL_EntityGetType(self._c_ampl, key.encode('utf-8'), &entitytype))
         if entitytype != self.entity_class: raiseKeyError(self.entity_class, key)
-        return m(self.entity_class, self._c_ampl, key, NULL)
+        return create_entity(self.entity_class, self._c_ampl, key, NULL)
 
     def size(self):
         return int(self._size)
@@ -123,6 +129,12 @@ cdef class InstanceIterator(object):
             instanceit.end = instanceit.begin + instanceit._size
         return instanceit
 
+    def __dealloc__(self):
+        if self.begin != NULL:
+            for i in range(self._size):
+                campl.AMPL_TupleFree(&self.begin[i])
+            free(self.begin)
+
     def __iter__(self):
         return self
 
@@ -131,15 +143,15 @@ cdef class InstanceIterator(object):
             raise StopIteration
         self.iterator += 1
         if self.begin == NULL:
-            return (None, m(self.entity_class, self._c_ampl, self._name, NULL))
+            return (None, create_entity(self.entity_class, self._c_ampl, self._name, NULL))
         else:
-            return (to_py_tuple(self.begin[self.iterator]), m(self.entity_class, self._c_ampl, self._name, self.begin[self.iterator]))
+            return (to_py_tuple(self.begin[self.iterator]), create_entity(self.entity_class, self._c_ampl, self._name, self.begin[self.iterator]))
 
     def __getitem__(self, key):
         assert isinstance(key, str)
         key = tuple(key)
         cdef campl.AMPL_TUPLE* tuple_c = to_c_tuple(key)
-        return m(self.entity_class, self._c_ampl, self._name, tuple_c)
+        return create_entity(self.entity_class, self._c_ampl, self._name, tuple_c)
 
     def size(self):
         return int(self._size)
@@ -173,6 +185,11 @@ cdef class MemberRangeIterator(object):
             instanceit.end = instanceit.begin + instanceit._size
         return instanceit
 
+    def __dealloc__(self):
+        for i in range(self._size):
+            campl.AMPL_TupleFree(&self.begin[i])
+        free(self.begin)
+
     def size(self):
         cdef size_t size
         campl.AMPL_SetInstanceGetSize(self._c_ampl, self._name.encode('utf-8'), self._index, &size)
@@ -250,4 +267,4 @@ cdef class RowIterator(object):
             raise StopIteration
         campl.AMPL_DataFrameElement(self._df, self._index, self._columnit, &v)
         self._columnit += 1
-        return to_py_variant(v)
+        return to_py_variant(v)
diff --git a/amplpy/util.pxi b/amplpy/util.pxi
@@ -122,7 +122,7 @@ cdef campl.AMPL_VARIANT* to_c_variant(value):
         raise ValueError(f"unsupported type {type(value)}")
     return variant
 
-cdef m(campl.AMPL_ENTITYTYPE entity_class, campl.AMPL* ampl, str name, campl.AMPL_TUPLE* index):
+cdef create_entity(campl.AMPL_ENTITYTYPE entity_class, campl.AMPL* ampl, str name, campl.AMPL_TUPLE* index):
     if entity_class == campl.AMPL_VARIABLE:
         return Variable.create(ampl, name, index)
     elif entity_class == campl.AMPL_CONSTRAINT: