f90wrap icon indicating copy to clipboard operation
f90wrap copied to clipboard

Memory error in recursive type initialization

Open bernardopacini opened this issue 5 years ago • 7 comments
trafficstars

Following up on the issue encountered with PR #131.

I believe the issue is specific to recursive types, so below is a simpler example (based on the recursive type array example) that shows the same problem.

Fortran:

module mod_recursive_type_array
  implicit none
  type t_node
    type(t_node),dimension(:),allocatable :: node
  end type t_node
  
contains

  subroutine allocate_node(root,N_node)
    type(t_node),intent(out) :: root
    integer,intent(in) :: N_node

    allocate(root%node(N_node))

  end subroutine allocate_node

  subroutine deallocate_node(root)
    type(t_node) :: root

    if (allocated(root%node)) deallocate(root%node)

  end subroutine
end module mod_recursive_type_array

Checked with the Python script:

import unittest
import numpy as np

from recursive_type_array import Mod_Recursive_Type_Array as recursive_type_array

class Test_recursive_type_array(unittest.TestCase):
    def setUp(self):
        self.N_node = 3

        self.root = recursive_type_array.t_node()

        self.root = recursive_type_array.allocate_node(self.N_node)
        self.root.node[0] = recursive_type_array.allocate_node(self.N_node)

    def tearDown(self):
        recursive_type_array.deallocate_node(self.root)

    def test_allocate(self):
        self.assertEqual(
            len(self.root.node),
            self.N_node
        )

    def test_deallocate(self):
        recursive_type_array.deallocate_node(self.root)
        self.assertEqual(
            len(self.root.node),
            0
        )

if __name__ == "__main__":
    unittest.main()

WIth this update, I run into the following error caused by the line self.root.node[0] = recursive_type_array.allocate_node(self.N_node):

Python(8902,0x1095b8dc0) malloc: *** error for object 0x3000000000000000: pointer being freed was not allocated
Python(8902,0x1095b8dc0) malloc: *** set a breakpoint in malloc_error_break to debug
[1]    8902 abort      python3 tests.py

When I use the previous commit (c4e8ea9bb2890844cdb62b4c752a365c3b699dcd) instead, the tests complete without an issue.

Thanks again!

bernardopacini avatar Nov 06 '20 17:11 bernardopacini

Hi again,

Changing the subroutine to be intent(inout) rather than intent(out) solves this issue, but I am not sure if this is a complete fix.

One could argue that for this specific case the subroutine should be inout as the node object already exists, but this may not be true in all cases.

Thanks

bernardopacini avatar Nov 06 '20 18:11 bernardopacini

This is a proposed fix, as explained in the discussion at #131

diff --git a/f90wrap/pywrapgen.py b/f90wrap/pywrapgen.py
index 3657ad0..4b95994 100644
--- a/f90wrap/pywrapgen.py
+++ b/f90wrap/pywrapgen.py
@@ -380,6 +380,7 @@ except ValueError:
         self.write('if self._alloc:')
         self.indent()
         self.write('%(mod_name)s.%(prefix)s%(func_name)s(%(f90_arg_names)s)' % dct)
+        self.write('self._alloc = False')
         self.dedent()
         self.dedent()
         self.write()

would you be willing to test this?

jameskermode avatar Nov 08 '20 08:11 jameskermode

Hi, 
Thank you for looking into this. I tried out the fix but I still run into the same segmentation fault. I am not sure if there is an issue with calling deallocate as the error occurs at the below line even when I remove any deallocate call from Python.

self.root.node[0] = recursive_type_array.allocate_node(self.N_node)

What is interesting is that calling the function once is okay and results in a new object self.root. It is the following call for adding in the node self.root.node[0] that causes the failure. You can also generate a totally new object self.root2 without issue.

Thank you again!

bernardopacini avatar Nov 09 '20 14:11 bernardopacini

Ok, thanks for testing. I’ll try to run it myself soon. I think it it still related to the automated destructor, but must be slightly more subtle.

jameskermode avatar Nov 09 '20 15:11 jameskermode

Got it -- thank you! Please let me know how I can help.

bernardopacini avatar Nov 09 '20 15:11 bernardopacini

Quick update: I've reproduced the issue, and hope to find a fix soon.

jameskermode avatar Nov 10 '20 14:11 jameskermode

Renaming the allocate and deallocate Fortran routines to node_allocate and node_deallocate respectively gets me a little further, as they are then recognised as the constructor and destructor for t_node objects and called automatically. There’s still a double free for the node stored within the array slot, which I’ll look into further.

jameskermode avatar Nov 10 '20 16:11 jameskermode